diff --git a/src/openjd/sessions/_session.py b/src/openjd/sessions/_session.py index 189984c3..cb051352 100644 --- a/src/openjd/sessions/_session.py +++ b/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,15 +466,11 @@ 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", @@ -479,14 +478,18 @@ def cleanup(self) -> None: "-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: diff --git a/src/openjd/sessions/_subprocess.py b/src/openjd/sessions/_subprocess.py index f9f02a3c..8def05ff 100644 --- a/src/openjd/sessions/_subprocess.py +++ b/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,6 +80,7 @@ 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") @@ -86,6 +88,8 @@ def __init__( 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(): diff --git a/test/openjd/sessions/test_runner_step_script.py b/test/openjd/sessions/test_runner_step_script.py index c3a2987e..f2458f1c 100644 --- a/test/openjd/sessions/test_runner_step_script.py +++ b/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", diff --git a/test/openjd/sessions/test_subprocess.py b/test/openjd/sessions/test_subprocess.py index 942d67c0..c6856eff 100644 --- a/test/openjd/sessions/test_subprocess.py +++ b/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: """