Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/openjd/sessions/_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions src/openjd/sessions/_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand All @@ -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]:
Expand Down Expand Up @@ -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():
Expand Down
4 changes: 3 additions & 1 deletion test/openjd/sessions/test_runner_step_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
26 changes: 20 additions & 6 deletions test/openjd/sessions/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
"""
Expand Down