fix: cleanup session dir on windows (#241)

Signed-off-by: Cody Edwards <edwards@amazon.com>

Author: edwards-aws

src/openjd/sessions/_session.py

@@ -48,6 +48,9 @@
 )
 from ._version import version
 
+if is_windows():  # pragma: nocover
+    from subprocess import HIGH_PRIORITY_CLASS  # type: ignore
+
 if TYPE_CHECKING:
     from openjd.model.v2023_09._model import EnvironmentVariableObject
 
@@ -463,30 +466,30 @@ def cleanup(self) -> None:
                 if self._user is not None:
                     files = [str(f) for f in self.working_directory.glob("*")]
 
+                    creation_flags = None
                     if is_posix():
                         recursive_delete_cmd = ["rm", "-rf"]
                     else:
                         recursive_delete_cmd = [
-                            "start",
-                            '"Powershell"',
-                            "/high",
-                            "/wait",
-                            "/b",
                             "powershell",
                             "-Command",
                             "Remove-Item",
                             "-Recurse",
                             "-Force",
                         ]
                         files = [", ".join(files)]
+                        # The cleanup needs to run as a high priority
+                        # https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getpriorityclass#return-value
+                        creation_flags = HIGH_PRIORITY_CLASS
 
-                    subprocess = LoggingSubprocess(
+                    _subprocess = LoggingSubprocess(
                         logger=self._logger,
                         args=recursive_delete_cmd + files,
                         user=self._user,
+                        creation_flags=creation_flags,
                     )
                     # Note: Blocking call until the process has exited
-                    subprocess.run()
+                    _subprocess.run()
 
                 self._working_dir.cleanup()
             except RuntimeError as exc:

src/openjd/sessions/_subprocess.py

@@ -64,6 +64,7 @@ class LoggingSubprocess(object):
     _has_started: Event
     _os_env_vars: Optional[dict[str, Optional[str]]]
     _working_dir: Optional[str]
+    _creation_flags: Optional[int]
 
     _pid: Optional[int]
     _sudo_child_process_group_id: Optional[int]
@@ -79,13 +80,16 @@ def __init__(
         callback: Optional[Callable[[], None]] = None,
         os_env_vars: Optional[dict[str, Optional[str]]] = None,
         working_dir: Optional[str] = None,
+        creation_flags: Optional[int] = None,
     ):
         if len(args) < 1:
             raise ValueError("'args' kwarg must be a sequence of at least one element")
         if user is not None and os.name == "posix" and not isinstance(user, PosixSessionUser):
             raise ValueError("Argument 'user' must be a PosixSessionUser on posix systems.")
         if user is not None and is_windows() and not isinstance(user, WindowsSessionUser):
             raise ValueError("Argument 'user' must be a WindowsSessionUser on Windows systems.")
+        if not is_windows() and creation_flags is not None:
+            raise ValueError("Argument 'creation_flags' is only supported on Windows")
 
         self._logger = logger
         self._args = args[:]  # Make a copy
@@ -100,6 +104,7 @@ def __init__(
         self._pid = None
         self._returncode = None
         self._sudo_child_process_group_id = None
+        self._creation_flags = creation_flags
 
     @property
     def pid(self) -> Optional[int]:
@@ -275,6 +280,9 @@ def _start_subprocess(self) -> Optional[Popen]:
                 # https://docs.python.org/2/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
                 popen_args["creationflags"] = CREATE_NEW_PROCESS_GROUP
 
+                if self._creation_flags:
+                    popen_args["creationflags"] |= self._creation_flags
+
             # Get the command string for logging
             cmd_line_for_logger: str
             if is_posix():

test/openjd/sessions/test_runner_step_script.py

@@ -309,7 +309,9 @@ def test_run_file_in_session_dir_as_windows_user(
         # GIVEN
         tmpdir = TempDir(user=windows_user)
         script = StepScript_2023_09(
-            actions=StepActions_2023_09(onRun=Action_2023_09(command=r"test.bat")),
+            actions=StepActions_2023_09(
+                onRun=Action_2023_09(command=CommandString_2023_09(r"test.bat"))
+            ),
             embeddedFiles=[
                 EmbeddedFileText_2023_09(
                     name="Foo",

test/openjd/sessions/test_subprocess.py

@@ -322,12 +322,14 @@ def test_terminate_ends_process_tree(
         all_messages = []
         # Note: This is the number of *CHILD* processes of the main process that we start.
         #  The total number of processes in flight will be this plus one.
-        expected_num_child_procs: int
-        if is_posix():
-            # Process tree: python -> python
-            # Children: python
-            expected_num_child_procs = 1
-        else:
+
+        # On Posix and on Windows not in a virutal environment:
+        # Process tree: python -> python
+        # Children: python
+        expected_num_child_procs = 1
+
+        # Check if we're in a virtual environment on Windows, see https://docs.python.org/3/library/venv.html#how-venvs-work
+        if is_windows() and sys.prefix != sys.base_prefix:
             # Windows starts an extra python process due to running in a virtual environment
             # Process tree: conhost -> python -> python -> python
             # Children: python, python, python
@@ -489,6 +491,18 @@ def test_run_gracetime_when_process_ends_but_grandchild_uses_stdout(
             m not in messages for m in not_expected_messages
         ), f"Unexpected messages: {', '.join(repr(m) for m in not_expected_messages if m in messages)}"
 
+    @pytest.mark.skipif(is_windows(), reason="Posix-specific tests")
+    def test_creation_flags_posix(self, queue_handler: QueueHandler) -> None:
+
+        with pytest.raises(ValueError):
+            logger = build_logger(queue_handler)
+            LoggingSubprocess(
+                logger=logger,
+                args=[sys.executable, "-c", 'print("this should not run")'],
+                # Creation flags aren't supported on Posix systems.
+                creation_flags=1337,
+            )
+
 
 def list_has_items_in_order(expected: list, actual: list) -> bool:
     """