From 1afb6e60df928da93741dba663185c41ff5765af Mon Sep 17 00:00:00 2001 From: dzerik Date: Wed, 1 Jul 2026 11:17:16 +0300 Subject: [PATCH 1/3] =?UTF-8?q?feat(python):=20streaming-stdio=20popen=20?= =?UTF-8?q?=E2=80=94=20Sandbox.popen=20+=20Process=20(RFC=20#67)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Python ergonomic layer over the C ABI sandlock_popen / sandlock_handle_kill: Sandbox.popen(cmd, stdin/stdout/stderr=StdioMode) returns a live Process whose PIPED streams are unbuffered binary file objects the caller reads/writes while the child runs; INHERIT/NULL streams are None. Streams default to INHERIT (parity with subprocess.Popen) — pass StdioMode.PIPED for the streams you drive. Process is a context manager that kills+reaps the child and closes the streams on exit (kill before wait, so an undrained pipe cannot hang teardown). wait() closes a still-open piped stdin first so a reader child (cat) sees EOF, then caches its Result (idempotent, no double-free). kill() SIGKILLs the group idempotently. On a failed fd handoff, Process.__init__ closes any fds it opened and popen() frees the handle — nothing leaks on the error path. - wait(timeout): bounds the wait via sandlock_handle_wait_timeout; on timeout the child is killed and a non-success Result is returned (parity with Sandbox.run's timeout), so an undrained-pipe wait can never hang forever. - __del__ safety net: a Process dropped without wait()/context reaps the child and emits a ResourceWarning (best-effort, never raises) instead of silently leaking the handle and wedging the Sandbox. - stdio modes are normalized/validated with StdioMode(x): a bad mode raises a clear ValueError in Python instead of an opaque null handle across the FFI; the int discriminant is still accepted. - Result from wait() carries no stdout/stderr (streamed to the fds) — documented on popen() and wait(). StdioMode/Process are exported from the package. Tests mirror the FFI suite and add destructively-verified coverage for the load-bearing invariants: stdout streaming, stdin/stdout roundtrip through cat, wait()-delivers-EOF to an unclosed piped stdin (watchdog), wait(timeout) kills-and-returns (watchdog), both stdout and stderr piped at once, stderr piped, Null yields no stream, kill lifecycle, context-manager reap + stream close + sandbox reuse, __del__ reaps an abandoned process, idempotent cached wait, int-discriminant accepted, unknown mode rejected, second-process guard. Refs #67 --- python/src/sandlock/__init__.py | 6 +- python/src/sandlock/_sdk.py | 21 +++ python/src/sandlock/sandbox.py | 264 +++++++++++++++++++++++++++++++- python/tests/test_popen.py | 239 +++++++++++++++++++++++++++++ 4 files changed, 528 insertions(+), 2 deletions(-) create mode 100644 python/tests/test_popen.py diff --git a/python/src/sandlock/__init__.py b/python/src/sandlock/__init__.py index d405ccfc..2711c3f0 100644 --- a/python/src/sandlock/__init__.py +++ b/python/src/sandlock/__init__.py @@ -14,7 +14,9 @@ ) from .inputs import inputs from .handler import Handler, NotifAction, HandlerCtx, ExceptionPolicy -from .sandbox import Sandbox, BranchAction, parse_ports, Change, DryRunResult +from .sandbox import ( + Sandbox, BranchAction, parse_ports, Change, DryRunResult, StdioMode, Process, +) from ._profile import load_profile, list_profiles from .exceptions import ( SandlockError, @@ -49,6 +51,8 @@ "parse_ports", "Change", "DryRunResult", + "StdioMode", + "Process", "Protection", # Handler ABI "Handler", diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index d55b6606..b158c024 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -317,6 +317,27 @@ def confine(policy: "PolicyDataclass") -> None: _lib.sandlock_handle_port_mappings.restype = ctypes.c_char_p _lib.sandlock_handle_port_mappings.argtypes = [_c_handle_p] +# Streaming-stdio popen (RFC #67): create+start a live handle with per-stream +# StdioMode; each piped stream's owned fd is returned through its out pointer. +_lib.sandlock_popen.restype = _c_handle_p +_lib.sandlock_popen.argtypes = [ + _c_policy_p, + ctypes.c_char_p, + ctypes.POINTER(ctypes.c_char_p), + ctypes.c_uint, + ctypes.c_uint32, # stdin_mode + ctypes.c_uint32, # stdout_mode + ctypes.c_uint32, # stderr_mode + ctypes.POINTER(ctypes.c_int), # out_stdin_fd + ctypes.POINTER(ctypes.c_int), # out_stdout_fd + ctypes.POINTER(ctypes.c_int), # out_stderr_fd +] + +# SIGKILL the handle's process group without freeing it (so wait can still +# collect the exit status). 0 on success, -1 on error. +_lib.sandlock_handle_kill.restype = ctypes.c_int +_lib.sandlock_handle_kill.argtypes = [_c_handle_p] + # Result _lib.sandlock_result_exit_code.restype = ctypes.c_int _lib.sandlock_result_exit_code.argtypes = [_c_result_p] diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index 1b8cd0a7..8b88b88b 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -11,7 +11,7 @@ import inspect import re from dataclasses import dataclass, field -from enum import Enum +from enum import Enum, IntEnum from typing import TYPE_CHECKING, Callable, Mapping, Sequence if TYPE_CHECKING: @@ -92,6 +92,20 @@ class BranchAction(Enum): KEEP = "keep" # Leave branch as-is (caller decides) +class StdioMode(IntEnum): + """Per-stream stdio wiring for :meth:`Sandbox.popen`. + + The values are the stable ABI discriminants shared with the C/Rust core. + """ + + INHERIT = 0 + """Share the supervisor's own fd (child writes to the same terminal/file).""" + PIPED = 1 + """Connect to a pipe; the caller owns the returned end via :class:`Process`.""" + NULL = 2 + """Connect the stream to ``/dev/null``.""" + + @dataclass(frozen=True) class Change: """A single filesystem change detected by dry-run.""" @@ -836,6 +850,85 @@ def wait(self): stderr=stderr, ) + def popen( + self, + cmd: Sequence[str], + stdin: StdioMode = StdioMode.INHERIT, + stdout: StdioMode = StdioMode.INHERIT, + stderr: StdioMode = StdioMode.INHERIT, + ) -> "Process": + """Spawn a confined process with per-stream stdio and return a live + :class:`Process` whose piped streams the caller reads/writes while it runs. + + The streaming counterpart of :meth:`run`: instead of buffering output + until the child exits, each stream set to :attr:`StdioMode.PIPED` is + handed back as a file object on the returned ``Process`` (``.stdin`` / + ``.stdout`` / ``.stderr``); ``INHERIT``/``NULL`` streams are ``None``. + Use it to drive a request/response protocol over stdio while the process + is alive (an MCP or LSP server, a REPL, JSON-RPC). + + All streams default to :attr:`StdioMode.INHERIT` (parity with + :class:`subprocess.Popen`); pass :attr:`StdioMode.PIPED` for exactly the + streams you want to drive. + + Deadlock warning (as with :class:`subprocess.Popen`): if you write to a + piped ``stdin`` you own it -- close it before :meth:`Process.wait` or a + child that reads to EOF (e.g. ``cat``) never exits and the wait blocks + forever; likewise drain a piped ``stdout``/``stderr`` before waiting or a + child that fills the pipe buffer blocks on write. ``Process`` is a + context manager that closes the streams and reaps the child on exit. + + The ``Result`` returned by :meth:`Process.wait` carries *no* captured + ``stdout``/``stderr`` (they were streamed to you as fds) — read the output + off ``proc.stdout``/``proc.stderr``, not off the result. + + Raises: + RuntimeError: If a process is already running, or the spawn failed. + ValueError: If a stdio argument is not a valid :class:`StdioMode`. + """ + import ctypes + from ._sdk import _lib, _make_argv + + if self._handle is not None: + raise RuntimeError("sandbox is already running") + + # Normalize/validate up front: StdioMode(x) accepts a StdioMode or its int + # discriminant and raises ValueError on anything else, so a bad mode fails + # clearly in Python instead of as an opaque null handle across the FFI. + stdin, stdout, stderr = StdioMode(stdin), StdioMode(stdout), StdioMode(stderr) + + native = self._ensure_native() + argv, argc = _make_argv(list(cmd)) + resolved_name = self._resolve_name() + + fd_in = ctypes.c_int(-1) + fd_out = ctypes.c_int(-1) + fd_err = ctypes.c_int(-1) + self._handle = _lib.sandlock_popen( + native.ptr, + _encode(resolved_name), + argv, + argc, + int(stdin), + int(stdout), + int(stderr), + ctypes.byref(fd_in), + ctypes.byref(fd_out), + ctypes.byref(fd_err), + ) + if not self._handle: + raise RuntimeError("sandlock_popen failed") + + try: + return Process(self, fd_in.value, fd_out.value, fd_err.value) + except BaseException: + # Wrapping the fds failed after the child was spawned. Process.__init__ + # already closed any fds it opened; free the handle (which reaps the + # child) and clear it so the Sandbox is left clean. + _lib.sandlock_handle_free(self._handle) + self._handle = None + raise + def dry_run(self, cmd: Sequence[str], timeout: float | None = None) -> "DryRunResult": """Dry-run: run a command, collect filesystem changes, then discard. @@ -1149,3 +1242,172 @@ def _resolve_syscall(key) -> int: f"syscall key must be a name (str) or number (int), " f"got {type(key).__name__}" ) + + +class Process: + """A live confined process with caller-owned stdio, returned by + :meth:`Sandbox.popen`. + + ``.stdin`` / ``.stdout`` / ``.stderr`` are unbuffered binary file objects + for streams opened with :attr:`StdioMode.PIPED`, else ``None``. The process + keeps running until :meth:`wait` (or :meth:`kill`) is called, or the + ``Process`` context manager exits. Prefer the context manager, which closes + the streams and reaps the child even on error:: + + with sandbox.popen(["cat"], stdin=StdioMode.PIPED, + stdout=StdioMode.PIPED) as proc: + proc.stdin.write(b"hi\\n") + proc.stdin.close() # EOF so cat exits + data = proc.stdout.read() + result = proc.wait() + """ + + def __init__(self, sandbox: "Sandbox", stdin_fd: int, stdout_fd: int, stderr_fd: int): + import os + + self._sandbox = sandbox + self._result = None + self.stdin = self.stdout = self.stderr = None + # os.fdopen takes ownership of each fd: closing the stream closes the fd, + # so a Process that is closed/exited never leaks the pipe ends. Wrap the + # three fds so that if a later fdopen raises, the streams already opened + # are closed and the not-yet-wrapped raw fds are closed too — no fd is + # leaked on the error path. + specs = ((stdin_fd, "wb"), (stdout_fd, "rb"), (stderr_fd, "rb")) + opened: list = [] + try: + for fd, mode in specs: + opened.append(os.fdopen(fd, mode, buffering=0) if fd >= 0 else None) + except BaseException: + for stream in opened: + if stream is not None: + try: + stream.close() + except OSError: + pass + for fd, _mode in specs[len(opened):]: + if fd >= 0: + try: + os.close(fd) + except OSError: + pass + raise + self.stdin, self.stdout, self.stderr = opened + + @property + def pid(self) -> int | None: + """The child PID while running, else ``None``.""" + return self._sandbox.pid + + def kill(self) -> None: + """SIGKILL the child's entire process group. Idempotent: a process that + already exited is not an error. The handle stays valid -- call + :meth:`wait` afterwards to collect the (non-success) exit status.""" + from ._sdk import _lib + + handle = self._sandbox._handle + if handle is None: + return + _lib.sandlock_handle_kill(handle) + + def wait(self, timeout: float | None = None) -> "Result": + """Wait for the child to exit and return its :class:`Result`. + + A still-open piped ``stdin`` is closed first so a child that reads to EOF + can exit; piped ``stdout``/``stderr`` stay open for the caller to finish + reading. Frees the underlying handle; a second call returns the cached + result. + + The returned ``Result`` carries no ``stdout``/``stderr`` (those were + streamed to you as fds — read them off ``.stdout``/``.stderr``). + + Args: + timeout: Maximum seconds to wait. On timeout the child is killed and + a non-success ``Result`` is returned (same semantics as + :meth:`Sandbox.run`'s ``timeout``) — the wait never hangs. ``None`` + (default) waits indefinitely. Note: without a ``timeout``, a piped + ``stdout``/``stderr`` you have not drained can block the child on a + full pipe and hang the wait forever; pass a ``timeout`` or drain first. + """ + from ._sdk import _lib, Result + + handle = self._sandbox._handle + if handle is None: + if self._result is not None: + return self._result + raise RuntimeError("process is not running") + + # Deliver EOF to a still-open piped stdin so a reader child can exit and + # the wait below does not block forever. + if self.stdin is not None and not self.stdin.closed: + try: + self.stdin.close() + except OSError: + pass + + try: + timeout_ms = int(timeout * 1000) if timeout else 0 + result_p = _lib.sandlock_handle_wait_timeout(handle, timeout_ms) + finally: + _lib.sandlock_handle_free(handle) + self._sandbox._handle = None + + if not result_p: + self._result = Result( + success=False, exit_code=-1, error="sandlock_handle_wait failed" + ) + return self._result + + exit_code = _lib.sandlock_result_exit_code(result_p) + success = _lib.sandlock_result_success(result_p) + # stdout/stderr were handed to the caller as fds, so the RunResult holds + # none — read them off the streams, not the Result. + _lib.sandlock_result_free(result_p) + self._result = Result(success=bool(success), exit_code=exit_code) + return self._result + + def __enter__(self) -> "Process": + return self + + def __exit__(self, exc_type, exc_val, exc_tb) -> None: + # Terminate and reap a still-running child so no confined process is left + # behind, then close every stream we own. + if self._sandbox._handle is not None: + try: + self.kill() + except Exception: + pass + try: + self.wait() + except Exception: + pass + for stream in (self.stdin, self.stdout, self.stderr): + if stream is not None and not stream.closed: + try: + stream.close() + except OSError: + pass + + def __del__(self): + # Safety net for a Process dropped without wait()/context: a live handle + # would otherwise leak the confined child + its runtime and wedge the + # Sandbox in "already running". Reap it, warning like subprocess.Popen so + # the missing wait()/`with` is visible. __del__ must never raise, and may + # run during interpreter shutdown when imports/globals are gone — so this + # is strictly best-effort inside a broad guard. + try: + if getattr(self, "_sandbox", None) is None or self._sandbox._handle is None: + return + import warnings + + warnings.warn( + "Process was not waited on or used as a context manager; " + "reaping the confined child. Use `with sandbox.popen(...)` or " + "call wait().", + ResourceWarning, + stacklevel=2, + ) + self.kill() + self.wait() + except Exception: + pass diff --git a/python/tests/test_popen.py b/python/tests/test_popen.py new file mode 100644 index 00000000..bbddf72e --- /dev/null +++ b/python/tests/test_popen.py @@ -0,0 +1,239 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Tests for the streaming-stdio Sandbox.popen / Process API (RFC #67). + +These drive real confined processes and read/write their pipe streams while +they run — the Python counterpart of crates/sandlock-ffi/tests/popen.rs. +""" + +from __future__ import annotations + +import sys +import threading + +import pytest + +from sandlock import Sandbox, StdioMode, Process + + +_READABLE = list(dict.fromkeys([ + "/usr", "/lib", "/lib64", "/bin", "/etc", "/proc", "/dev", sys.prefix, +])) + + +def _policy(**overrides): + defaults = {"fs_readable": _READABLE} + defaults.update(overrides) + return Sandbox(**defaults) + + +def _sandbox_works() -> bool: + """Probe whether a sandbox can actually start on this host (Landlock/ABI).""" + try: + return bool(_policy().run(["true"]).success) + except Exception: + return False + + +pytestmark = pytest.mark.skipif( + not _sandbox_works(), reason="sandbox cannot start on this host (Landlock/ABI)" +) + + +def test_popen_streams_stdout_and_collects_exit(): + proc = _policy().popen(["echo", "ffi-hi"], stdout=StdioMode.PIPED) + assert proc.stdin is None, "stdin inherited → no stream" + assert proc.stdout is not None, "stdout piped → stream present" + assert proc.stderr is None, "stderr inherited → no stream" + + assert proc.stdout.read() == b"ffi-hi\n" + result = proc.wait() + assert result.success + assert result.exit_code == 0 + + +def test_popen_stdin_stdout_roundtrip(): + proc = _policy().popen(["cat"], stdin=StdioMode.PIPED, stdout=StdioMode.PIPED) + assert proc.stdin is not None and proc.stdout is not None + + proc.stdin.write(b"ping\n") + proc.stdin.close() # EOF → cat exits + assert proc.stdout.read() == b"ping\n" + + result = proc.wait() + assert result.success + assert result.exit_code == 0 + + +def test_wait_closes_unclosed_piped_stdin_so_reader_exits(): + # cat reads stdin to EOF. If the caller pipes stdin but never closes it, + # wait() must close it to deliver EOF — otherwise cat blocks forever and + # wait() (no timeout) hangs. Run wait() under a watchdog so a regression + # surfaces as a failure, not a hung CI job. + proc = _policy().popen(["cat"], stdin=StdioMode.PIPED, stdout=StdioMode.PIPED) + proc.stdin.write(b"data\n") + # Deliberately DO NOT close stdin here — wait() is responsible for the EOF. + + box = {} + + def _run(): + box["result"] = proc.wait() + + t = threading.Thread(target=_run, daemon=True) + t.start() + t.join(timeout=10) + if t.is_alive(): + proc.kill() # unblock the child so the worker thread can unwind + t.join(timeout=5) + pytest.fail("wait() did not close unclosed piped stdin → cat blocked forever") + + assert box["result"].success + assert box["result"].exit_code == 0 + + +def test_popen_stderr_piped(): + proc = _policy().popen( + ["sh", "-c", "echo err 1>&2"], + stdout=StdioMode.INHERIT, + stderr=StdioMode.PIPED, + ) + assert proc.stdout is None, "stdout inherited → no stream" + assert proc.stderr is not None, "stderr piped → stream present" + + assert proc.stderr.read() == b"err\n" + assert proc.wait().success + + +def test_popen_null_stdout_yields_no_stream(): + proc = _policy().popen(["echo", "discarded"], stdout=StdioMode.NULL) + assert proc.stdout is None, "Null stdout → no caller stream" + assert proc.wait().exit_code == 0 + + +def test_popen_kill_then_wait_is_not_success(): + proc = _policy().popen(["sleep", "100"]) + proc.kill() + result = proc.wait() + assert not result.success, "a killed process is not success" + + +def test_popen_kill_is_idempotent(): + proc = _policy().popen(["sleep", "100"]) + proc.kill() + proc.kill() # second kill on a dying/exited process must not raise + proc.wait() + + +def test_process_context_manager_reaps_child_and_closes_streams(): + sandbox = _policy() + with sandbox.popen( + ["sleep", "100"], stdout=StdioMode.PIPED + ) as proc: + assert proc.pid is not None + assert sandbox.is_running + stdout = proc.stdout + # Leaving the block must terminate + reap the child and close every stream. + assert not sandbox.is_running, "context exit must reap the child" + assert proc.pid is None + assert stdout.closed, "context exit must close piped streams" + # The sandbox is reusable after the process is reaped. + assert _policy_reusable(sandbox) + + +def _policy_reusable(sandbox) -> bool: + result = sandbox.run(["true"]) + return bool(result.success) + + +def test_wait_is_idempotent_and_cached(): + proc = _policy().popen(["echo", "hi"], stdout=StdioMode.PIPED) + assert proc.stdout.read() == b"hi\n" + first = proc.wait() + second = proc.wait() # handle already freed → cached result, no double-free + assert first is second + assert first.exit_code == 0 + + +def test_popen_unknown_stdio_mode_raises(): + # An out-of-range discriminant is rejected in Python (StdioMode(99)) before + # crossing the FFI — a clear ValueError, no opaque null handle. + with pytest.raises(ValueError): + _policy().popen(["echo", "x"], stdout=99) # type: ignore[arg-type] + + +def test_popen_accepts_int_discriminant_for_stdio_mode(): + # The int discriminant is accepted (normalized to StdioMode) for callers that + # pass a raw 0/1/2 rather than the enum. + proc = _policy().popen(["echo", "raw"], stdout=1) # 1 == StdioMode.PIPED + assert proc.stdout is not None + assert proc.stdout.read() == b"raw\n" + assert proc.wait().success + + +def test_popen_rejects_second_process_on_same_sandbox(): + sandbox = _policy() + proc = sandbox.popen(["sleep", "100"]) + try: + with pytest.raises(RuntimeError): + sandbox.popen(["echo", "x"]) + finally: + proc.kill() + proc.wait() + + +def test_wait_timeout_kills_and_returns_nonsuccess(): + # A child that never exits within the timeout: wait(timeout) must RETURN (not + # hang), killing the child and yielding a non-success result — parity with + # Sandbox.run(timeout=...). Run under a watchdog so a regression (ignored + # timeout → hang) surfaces as a failure, not a stuck job. + proc = _policy().popen(["sleep", "100"]) + + box = {} + + def _run(): + box["result"] = proc.wait(timeout=0.5) + + t = threading.Thread(target=_run, daemon=True) + t.start() + t.join(timeout=10) + if t.is_alive(): + proc.kill() + t.join(timeout=5) + pytest.fail("wait(timeout) did not return — timeout was ignored") + + assert not box["result"].success, "a timed-out (killed) process is not success" + + +def test_wait_without_timeout_completes_for_quick_child(): + # timeout=None keeps the indefinite-wait behavior for a child that exits. + proc = _policy().popen(["echo", "quick"], stdout=StdioMode.PIPED) + assert proc.stdout.read() == b"quick\n" + assert proc.wait().success + + +def test_popen_stdout_and_stderr_both_piped(): + # Both streams piped at once: each is handed back and drained independently. + proc = _policy().popen( + ["sh", "-c", "echo out; echo err 1>&2"], + stdout=StdioMode.PIPED, + stderr=StdioMode.PIPED, + ) + assert proc.stdin is None + assert proc.stdout is not None and proc.stderr is not None + assert proc.stdout.read() == b"out\n" + assert proc.stderr.read() == b"err\n" + assert proc.wait().success + + +def test_abandoned_process_is_reaped_by_del(): + # A Process dropped without wait()/context must not leak: __del__ reaps the + # child (freeing the handle) and warns. Assert both the ResourceWarning and + # that the sandbox is released for reuse. + import gc + + sandbox = _policy() + with pytest.warns(ResourceWarning): + proc = sandbox.popen(["sleep", "100"]) + del proc + gc.collect() + assert not sandbox.is_running, "__del__ must reap an abandoned child" + assert _policy_reusable(sandbox), "sandbox is reusable after __del__ reap" From 3ac2df612c2734d20380caa17b3599221c8345ea Mon Sep 17 00:00:00 2001 From: dzerik Date: Sat, 4 Jul 2026 12:17:40 +0300 Subject: [PATCH 2/3] Address review: Process owns its handle; fix wait timeout, kill, sb-method aliasing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handle ownership (load-bearing): a popen() Process now owns its handle for its lifetime instead of resolving sandbox._handle at call time. Previously every Process method (kill/wait/__del__/pid) read the sandbox's current handle, so a stale Process aliased whatever child the sandbox ran next: `p1.kill()` after the sandbox was reused SIGKILLed p2, and a stale `p1.wait()` reaped p2 and left it "not running". - Process takes and stores the handle (and caches its pid); all methods act on self._handle and set it to None once freed. The handle/pid are taken only AFTER the fds are wrapped, so an fdopen failure leaves ownership with popen()'s caller — no double free via __del__. - Sandbox keeps only a weak busy marker (self._process, a weakref) so the one-process-per-sandbox rule and pid/is_running/ports still see the live child, while abandoning the Process still triggers its __del__ reap rather than the sandbox pinning it alive. - Sandbox.wait/pause/resume/kill fail loudly when a popen() Process owns the handle (manage it through the Process). Concurrency: Process guards its handle with a lock and a `waiting` flag. wait() runs the blocking native wait without the lock; kill() signals the process group by cached pid (like the sandbox and the Go binding), not through the handle, so the documented "kill from another thread while wait() blocks" pattern is race-free — no kill on a handle wait() is freeing. kill() raises OSError on a genuine failure (ProcessLookupError, i.e. already-exited, is a no-op) and is a silent no-op once reaped. Also: Process.wait / Sandbox.run clamp a finite timeout up to 1ms so `timeout=0` and sub-millisecond values don't collapse to 0 (= wait forever); None still waits forever. Tests: stale-Process-does-not-alias-reused-sandbox; sandbox lifecycle methods reject a popen Process; wait(timeout=0) returns instead of hanging; kill from another thread interrupts a blocked wait. --- python/src/sandlock/sandbox.py | 222 ++++++++++++++++++++++++++------- python/tests/test_popen.py | 85 +++++++++++++ 2 files changed, 259 insertions(+), 48 deletions(-) diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index 8b88b88b..acc46823 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -10,6 +10,7 @@ import inspect import re +import weakref from dataclasses import dataclass, field from enum import Enum, IntEnum from typing import TYPE_CHECKING, Callable, Mapping, Sequence @@ -419,6 +420,9 @@ def __post_init__(self): # Runtime state — not dataclass fields, not serialized self._native = None # _NativePolicy created lazily on first use self._handle = None # live sandbox handle during start()/run() + self._process = None # weakref to the live popen() Process; it OWNS its + # own handle (see `_popen_process`), this is only a + # non-owning busy marker def _resolve_name(self) -> str: """Resolve sandbox name: explicit > auto-generated.""" @@ -485,10 +489,60 @@ def cpu_pct(self) -> int | None: # Context manager # ------------------------------------------------------------------ + def _popen_process(self): + """The live :meth:`popen` :class:`Process`, or ``None``. Held *weakly*: + this is only a busy marker, so abandoning the ``Process`` still lets its + ``__del__`` reap the child rather than the sandbox pinning it alive.""" + ref = self._process + return ref() if ref is not None else None + + def _live_handle(self): + """The handle owning the currently running child, from either source: + the ``create``/``start``/``run``/``spawn`` lifecycle (``self._handle``) or + a :meth:`popen` :class:`Process` (which owns its handle). Returns ``None`` + when nothing is running. This is the single source of truth for "busy".""" + if self._handle is not None: + return self._handle + proc = self._popen_process() + if proc is not None and proc._handle is not None: + return proc._handle + return None + + def _check_not_running(self) -> None: + """Raise if any child is live, so a second lifecycle call can't leak the + first handle or alias a running popen() child.""" + if self._live_handle() is not None: + raise RuntimeError("sandbox is already running") + + def _reject_if_popen(self) -> None: + """Raise if the live child is driven by a :meth:`popen` :class:`Process`. + The sandbox's own lifecycle methods (``wait``/``pause``/``resume``/``kill``) + must not touch a handle the ``Process`` owns — freeing or signalling it + behind the ``Process``'s back is exactly the aliasing this split avoids.""" + proc = self._popen_process() + if proc is not None and proc._handle is not None: + raise RuntimeError( + "this sandbox is running a process started with popen(); manage it " + "through the returned Process (proc.wait() / proc.kill()), not the " + "sandbox" + ) + def __enter__(self): return self def __exit__(self, exc_type, exc_val, exc_tb): + # Reap a still-running popen() Process (it owns its own handle) so leaving + # the sandbox context never strands a confined child. + proc = self._popen_process() + if proc is not None and proc._handle is not None: + try: + proc.kill() + except Exception: + pass + try: + proc.wait() + except Exception: + pass if self._handle is not None: from ._sdk import _lib try: @@ -504,16 +558,19 @@ def __exit__(self, exc_type, exc_val, exc_tb): @property def pid(self) -> int | None: - """Child PID while running, None otherwise.""" - if self._handle is None: + """Child PID while running, None otherwise. Reflects a running child from + either the sandbox lifecycle or a live :meth:`popen` :class:`Process`.""" + handle = self._live_handle() + if handle is None: return None from ._sdk import _lib - return _lib.sandlock_handle_pid(self._handle) or None + return _lib.sandlock_handle_pid(handle) or None @property def is_running(self) -> bool: - """True if a process is currently running in this sandbox.""" - return self._handle is not None + """True if a process is currently running in this sandbox (lifecycle or + a live :meth:`popen` :class:`Process`).""" + return self._live_handle() is not None # ------------------------------------------------------------------ # Execution methods @@ -536,8 +593,7 @@ def run(self, cmd: Sequence[str], timeout: float | None = None): """ from ._sdk import _lib, _make_argv, _read_result_bytes, Result - if self._handle is not None: - raise RuntimeError("sandbox is already running") + self._check_not_running() native = self._ensure_native() argv, argc = _make_argv(list(cmd)) @@ -558,7 +614,9 @@ def run(self, cmd: Sequence[str], timeout: float | None = None): return Result(success=False, exit_code=-1, error="sandlock_start failed") try: - timeout_ms = int(timeout * 1000) if timeout else 0 + # None -> wait forever (0). A finite timeout clamps up to 1ms so + # timeout=0 / sub-ms don't collapse to 0 (= wait forever). + timeout_ms = max(1, int(timeout * 1000)) if timeout is not None else 0 result_p = _lib.sandlock_handle_wait_timeout(self._handle, timeout_ms) finally: _lib.sandlock_handle_free(self._handle) @@ -631,8 +689,7 @@ def run_with_handlers( Result, ) - if self._handle is not None: - raise RuntimeError("sandbox is already running") + self._check_not_running() # Resolve syscall keys (str name -> host-arch number, int as is) # up front: an unknown name fails loudly here, before any native @@ -775,8 +832,7 @@ def create(self, cmd: Sequence[str]) -> None: """ from ._sdk import _lib, _make_argv - if self._handle is not None: - raise RuntimeError("sandbox is already running") + self._check_not_running() native = self._ensure_native() argv, argc = _make_argv(list(cmd)) @@ -821,10 +877,13 @@ def wait(self): """Wait for the running process to finish and return its Result. Raises: - RuntimeError: If the sandbox is not running. + RuntimeError: If the sandbox is not running, or if it is running a + :meth:`popen` :class:`Process` (wait on that Process instead — + freeing its handle here would break it). """ from ._sdk import _lib, _read_result_bytes, Result + self._reject_if_popen() if self._handle is None: raise RuntimeError("sandbox is not running") @@ -889,8 +948,7 @@ def popen( import ctypes from ._sdk import _lib, _make_argv - if self._handle is not None: - raise RuntimeError("sandbox is already running") + self._check_not_running() # Normalize/validate up front: StdioMode(x) accepts a StdioMode or its int # discriminant and raises ValueError on anything else, so a bad mode fails @@ -904,7 +962,11 @@ def popen( fd_in = ctypes.c_int(-1) fd_out = ctypes.c_int(-1) fd_err = ctypes.c_int(-1) - self._handle = _lib.sandlock_popen( + # The handle is owned by the returned Process, not the Sandbox: it is + # never stored in self._handle. self._process (set below) is the busy + # marker (`_live_handle`), so a stale Process can never alias a later + # child through the sandbox. + handle = _lib.sandlock_popen( native.ptr, _encode(resolved_name), argv, @@ -916,18 +978,19 @@ def popen( ctypes.byref(fd_out), ctypes.byref(fd_err), ) - if not self._handle: + if not handle: raise RuntimeError("sandlock_popen failed") try: - return Process(self, fd_in.value, fd_out.value, fd_err.value) + proc = Process(self, handle, fd_in.value, fd_out.value, fd_err.value) except BaseException: # Wrapping the fds failed after the child was spawned. Process.__init__ # already closed any fds it opened; free the handle (which reaps the - # child) and clear it so the Sandbox is left clean. - _lib.sandlock_handle_free(self._handle) - self._handle = None + # child) so the Sandbox is left clean and reusable. + _lib.sandlock_handle_free(handle) raise + self._process = weakref.ref(proc) + return proc def dry_run(self, cmd: Sequence[str], timeout: float | None = None) -> "DryRunResult": """Dry-run: run a command, collect filesystem changes, then discard. @@ -1096,8 +1159,12 @@ def reduce(self, cmd: list[str], fork_result: "ForkResult") -> "Result": # ------------------------------------------------------------------ def pause(self) -> None: - """Send SIGSTOP to the sandbox process group.""" + """Send SIGSTOP to the sandbox process group. + + Raises RuntimeError if a :meth:`popen` Process owns the child (manage it + through the Process).""" import signal + self._reject_if_popen() pid = self.pid if pid is None: raise RuntimeError("sandbox is not running") @@ -1105,8 +1172,11 @@ def pause(self) -> None: os.killpg(pid, signal.SIGSTOP) def resume(self) -> None: - """Send SIGCONT to the sandbox process group.""" + """Send SIGCONT to the sandbox process group. + + Raises RuntimeError if a :meth:`popen` Process owns the child.""" import signal + self._reject_if_popen() pid = self.pid if pid is None: raise RuntimeError("sandbox is not running") @@ -1114,8 +1184,12 @@ def resume(self) -> None: os.killpg(pid, signal.SIGCONT) def kill(self) -> None: - """Send SIGKILL to the sandbox process group.""" + """Send SIGKILL to the sandbox process group. + + Raises RuntimeError if a :meth:`popen` Process owns the child (call + ``proc.kill()`` instead).""" import signal + self._reject_if_popen() pid = self.pid if pid is None: raise RuntimeError("sandbox is not running") @@ -1130,12 +1204,14 @@ def ports(self) -> dict[int, int]: Only contains entries where the real port differs from the virtual port (i.e., where a remap occurred). Empty if port_remap is disabled - or no ports have been remapped. Requires the sandbox to be running. + or no ports have been remapped. Works for a running child from either + the lifecycle or a live :meth:`popen` :class:`Process`. """ - if self._handle is None: + handle = self._live_handle() + if handle is None: return {} from ._sdk import _lib - c_str = _lib.sandlock_handle_port_mappings(self._handle) + c_str = _lib.sandlock_handle_port_mappings(handle) if not c_str: return {} try: @@ -1262,10 +1338,22 @@ class Process: result = proc.wait() """ - def __init__(self, sandbox: "Sandbox", stdin_fd: int, stdout_fd: int, stderr_fd: int): + def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, stderr_fd: int): import os + import threading + from ._sdk import _lib self._sandbox = sandbox + # _lock guards _handle (and the _waiting reservation) so kill()/pid on + # one thread can't observe or act on a handle wait() is freeing on + # another — the "kill from another thread while wait() blocks" pattern. + self._lock = threading.Lock() + self._waiting = False + # The handle and pid are set only AFTER the fds are wrapped below: if an + # fdopen raises, this Process never takes ownership, so popen()'s caller + # frees the handle exactly once (no double free via __del__). + self._handle = None + self._pid = -1 self._result = None self.stdin = self.stdout = self.stderr = None # os.fdopen takes ownership of each fd: closing the stream closes the fd, @@ -1293,22 +1381,43 @@ def __init__(self, sandbox: "Sandbox", stdin_fd: int, stdout_fd: int, stderr_fd: pass raise self.stdin, self.stdout, self.stderr = opened + # Take ownership now that construction can no longer fail. Cache the pid + # so kill()/pid never dereference the handle (avoiding a race with a + # concurrent wait() free) — they signal by pid, like the sandbox and the + # Go binding. + self._pid = _lib.sandlock_handle_pid(handle) or -1 + self._handle = handle @property def pid(self) -> int | None: - """The child PID while running, else ``None``.""" - return self._sandbox.pid + """The child PID while running, else ``None`` (after :meth:`wait`).""" + with self._lock: + return self._pid if self._handle is not None and self._pid > 0 else None def kill(self) -> None: - """SIGKILL the child's entire process group. Idempotent: a process that - already exited is not an error. The handle stays valid -- call - :meth:`wait` afterwards to collect the (non-success) exit status.""" - from ._sdk import _lib + """SIGKILL the child's entire process group by pid (like the sandbox and + the Go binding), so a kill from another thread is safe while :meth:`wait` + blocks on the handle. Idempotent: a child that already exited is not an + error, and after :meth:`wait` has reaped it this is a no-op. - handle = self._sandbox._handle - if handle is None: - return - _lib.sandlock_handle_kill(handle) + Raises: + RuntimeError: If the process has no pid (never a valid popen child). + OSError: If the kill genuinely fails (e.g. EPERM) — a + ``ProcessLookupError`` (already-exited group) is swallowed. + """ + import os + import signal + + with self._lock: + if self._handle is None: + return # already reaped by wait() — no-op + if self._pid <= 0: + raise RuntimeError("process has no pid") + pid = self._pid + try: + os.killpg(pid, signal.SIGKILL) + except ProcessLookupError: + pass # the group already exited — idempotent no-op def wait(self, timeout: float | None = None) -> "Result": """Wait for the child to exit and return its :class:`Result`. @@ -1331,11 +1440,18 @@ def wait(self, timeout: float | None = None) -> "Result": """ from ._sdk import _lib, Result - handle = self._sandbox._handle - if handle is None: - if self._result is not None: - return self._result - raise RuntimeError("process is not running") + # Reserve the handle under the lock so a concurrent kill()/pid sees a + # consistent state, then run the blocking wait WITHOUT the lock so kill() + # (which signals by pid, not through the handle) can interrupt it. + with self._lock: + handle = self._handle + if handle is None: + if self._result is not None: + return self._result + raise RuntimeError("process is not running") + if self._waiting: + raise RuntimeError("wait already in progress") + self._waiting = True # Deliver EOF to a still-open piped stdin so a reader child can exit and # the wait below does not block forever. @@ -1346,11 +1462,21 @@ def wait(self, timeout: float | None = None) -> "Result": pass try: - timeout_ms = int(timeout * 1000) if timeout else 0 + # None -> wait forever (the FFI treats 0 as "no timeout"). A finite + # timeout maps to milliseconds, clamped up to 1ms so timeout=0 and + # sub-millisecond values don't collapse to 0 and wait forever — the + # opposite of the caller's intent. + timeout_ms = max(1, int(timeout * 1000)) if timeout is not None else 0 result_p = _lib.sandlock_handle_wait_timeout(handle, timeout_ms) finally: - _lib.sandlock_handle_free(handle) - self._sandbox._handle = None + with self._lock: + _lib.sandlock_handle_free(handle) + self._handle = None + self._waiting = False + # Release the sandbox's busy marker so it can be reused. Guard that it + # still points at us (the weakref may already be dead during __del__). + if self._sandbox._popen_process() is self: + self._sandbox._process = None if not result_p: self._result = Result( @@ -1372,7 +1498,7 @@ def __enter__(self) -> "Process": def __exit__(self, exc_type, exc_val, exc_tb) -> None: # Terminate and reap a still-running child so no confined process is left # behind, then close every stream we own. - if self._sandbox._handle is not None: + if self._handle is not None: try: self.kill() except Exception: @@ -1396,7 +1522,7 @@ def __del__(self): # run during interpreter shutdown when imports/globals are gone — so this # is strictly best-effort inside a broad guard. try: - if getattr(self, "_sandbox", None) is None or self._sandbox._handle is None: + if getattr(self, "_handle", None) is None: return import warnings diff --git a/python/tests/test_popen.py b/python/tests/test_popen.py index bbddf72e..11273644 100644 --- a/python/tests/test_popen.py +++ b/python/tests/test_popen.py @@ -9,6 +9,7 @@ import sys import threading +import time import pytest @@ -224,6 +225,90 @@ def test_popen_stdout_and_stderr_both_piped(): assert proc.wait().success +def test_kill_from_another_thread_interrupts_blocked_wait(): + # The documented pattern: wait() blocks in one thread while kill() fires from + # another. kill() signals by pid (not through the handle), so it interrupts + # the blocked wait without racing wait()'s handle free. Watchdog so a + # deadlock/hang surfaces as a failure, not a stuck job. + proc = _policy().popen(["sleep", "100"]) + + box = {} + + def _run(): + box["result"] = proc.wait() # no timeout: only kill() can end this + + t = threading.Thread(target=_run, daemon=True) + t.start() + time.sleep(0.3) # let wait() reach the blocking native wait + proc.kill() + t.join(timeout=5) + if t.is_alive(): + pytest.fail("kill() from another thread did not interrupt a blocked wait()") + + assert not box["result"].success, "a killed process is not success" + + +def test_stale_process_does_not_alias_reused_sandbox(): + # The load-bearing invariant of Process owning its handle: after a Process is + # waited and the sandbox reused, the stale Process's kill()/wait() must not + # touch the NEW child. Previously both resolved sandbox._handle at call time, + # so p1.kill() would SIGKILL p2 and a stale p1.wait() would reap it. + sandbox = _policy() + p1 = sandbox.popen(["echo", "one"], stdout=StdioMode.PIPED) + assert p1.stdout.read() == b"one\n" + p1.wait() # frees p1's handle, releases the sandbox + + p2 = sandbox.popen(["sleep", "100"]) + try: + assert sandbox.is_running + p1.kill() # no-op on the reaped p1 — must NOT kill p2 + p1.wait() # cached result — must NOT reap p2 + assert sandbox.is_running, "stale p1 must not have reaped p2" + assert p2.pid is not None, "p2 must still be alive" + finally: + p2.kill() + p2.wait() + assert not sandbox.is_running + + +def test_sandbox_lifecycle_methods_reject_popen_process(): + # While a popen() Process owns the handle, the sandbox's own lifecycle + # methods must fail loudly instead of half-working / freeing behind its back. + sandbox = _policy() + proc = sandbox.popen(["sleep", "100"]) + try: + for op in (sandbox.wait, sandbox.kill, sandbox.pause, sandbox.resume): + with pytest.raises(RuntimeError): + op() + # The Process still works after the rejected sandbox-level calls. + assert proc.pid is not None + finally: + proc.kill() + proc.wait() + + +def test_wait_timeout_zero_does_not_wait_forever(): + # timeout=0 (and sub-millisecond values) must not collapse to "wait forever"; + # they return promptly with a non-success (killed) result. Watchdog so a + # regression surfaces as a failure, not a hang. + proc = _policy().popen(["sleep", "100"]) + + box = {} + + def _run(): + box["result"] = proc.wait(timeout=0) + + t = threading.Thread(target=_run, daemon=True) + t.start() + t.join(timeout=10) + if t.is_alive(): + proc.kill() + t.join(timeout=5) + pytest.fail("wait(timeout=0) hung — 0 collapsed to wait-forever") + + assert not box["result"].success + + def test_abandoned_process_is_reaped_by_del(): # A Process dropped without wait()/context must not leak: __del__ reaps the # child (freeing the handle) and warns. Assert both the ResourceWarning and From e26e9d01ef6ee31103673402dc5054bdfb50c073 Mon Sep 17 00:00:00 2001 From: dzerik Date: Sun, 5 Jul 2026 00:55:49 +0300 Subject: [PATCH 3/3] Address review round 2: checkpoint reject, thread-safe pid/ports, drop dead binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - checkpoint(): a popen() Process owns the handle, so checkpoint (SIGSTOP + ptrace freeze) now rejects with _reject_if_popen() instead of reaching past the Process for a handle that is None on the sandbox — consistent with wait/pause/resume/kill. - Sandbox.pid / Sandbox.ports: delegate to the Process (Process.pid is lock-guarded + cached; new Process.ports() reads the handle under the lock and reports {} while a wait() holds it). Previously both did a raw FFI read of the Process's handle via _live_handle(), racing a concurrent wait() free. Extracted _read_port_mappings() shared by both. - Removed the now-unused sandlock_handle_kill ctypes binding: Process.kill signals the group by pid (killpg), so the FFI kill has no caller. Tests: checkpoint() rejects a popen Process; Sandbox.pid/ports delegate to the live Process and return None/{} after it is reaped. --- python/src/sandlock/_sdk.py | 5 --- python/src/sandlock/sandbox.py | 58 +++++++++++++++++++++++++--------- python/tests/test_popen.py | 20 +++++++++++- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index b158c024..c1759956 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -333,11 +333,6 @@ def confine(policy: "PolicyDataclass") -> None: ctypes.POINTER(ctypes.c_int), # out_stderr_fd ] -# SIGKILL the handle's process group without freeing it (so wait can still -# collect the exit status). 0 on success, -1 on error. -_lib.sandlock_handle_kill.restype = ctypes.c_int -_lib.sandlock_handle_kill.argtypes = [_c_handle_p] - # Result _lib.sandlock_result_exit_code.restype = ctypes.c_int _lib.sandlock_result_exit_code.argtypes = [_c_result_p] diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index acc46823..5ce9b290 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -560,11 +560,15 @@ def __exit__(self, exc_type, exc_val, exc_tb): def pid(self) -> int | None: """Child PID while running, None otherwise. Reflects a running child from either the sandbox lifecycle or a live :meth:`popen` :class:`Process`.""" - handle = self._live_handle() - if handle is None: + proc = self._popen_process() + if proc is not None: + # The Process owns the handle; its pid is lock-guarded and cached, so + # reading it can't race a concurrent wait() freeing the handle. + return proc.pid + if self._handle is None: return None from ._sdk import _lib - return _lib.sandlock_handle_pid(handle) or None + return _lib.sandlock_handle_pid(self._handle) or None @property def is_running(self) -> bool: @@ -1207,19 +1211,14 @@ def ports(self) -> dict[int, int]: or no ports have been remapped. Works for a running child from either the lifecycle or a live :meth:`popen` :class:`Process`. """ - handle = self._live_handle() - if handle is None: - return {} - from ._sdk import _lib - c_str = _lib.sandlock_handle_port_mappings(handle) - if not c_str: + proc = self._popen_process() + if proc is not None: + # The Process owns the handle; its ports() reads it under the lock so + # this can't race a concurrent wait() freeing it. + return proc.ports() + if self._handle is None: return {} - try: - import json - raw = json.loads(c_str.decode()) - return {int(k): v for k, v in raw.items()} - finally: - _lib.sandlock_string_free(c_str) + return _read_port_mappings(self._handle) def checkpoint( self, @@ -1244,6 +1243,10 @@ def checkpoint( """ from ._sdk import _lib, Checkpoint + # A popen() Process owns its handle; checkpoint (SIGSTOP + ptrace freeze) + # would fight the streaming Process for it, so reject rather than reach + # past it — consistent with wait/pause/resume/kill. + self._reject_if_popen() if self._handle is None: raise RuntimeError("sandbox is not running (use start() or run() first)") ptr = _lib.sandlock_handle_checkpoint(self._handle) @@ -1255,6 +1258,22 @@ def checkpoint( return cp +def _read_port_mappings(handle) -> dict[int, int]: + """Decode {virtual: real} port mappings for a live handle. Shared by + ``Sandbox.ports`` and ``Process.ports``; the caller must hold whatever lock + guards ``handle`` while invoking this.""" + from ._sdk import _lib + c_str = _lib.sandlock_handle_port_mappings(handle) + if not c_str: + return {} + try: + import json + raw = json.loads(c_str.decode()) + return {int(k): v for k, v in raw.items()} + finally: + _lib.sandlock_string_free(c_str) + + def _encode(s: str) -> bytes: """Encode a string to UTF-8 bytes, rejecting NUL bytes.""" if isinstance(s, str): @@ -1394,6 +1413,15 @@ def pid(self) -> int | None: with self._lock: return self._pid if self._handle is not None and self._pid > 0 else None + def ports(self) -> dict[int, int]: + """Current virtual→real port mappings while running, else ``{}`` (needs + ``port_remap``). Reads the handle under the lock, and reports ``{}`` + while a :meth:`wait` holds it, so it can't race the wait's free.""" + with self._lock: + if self._handle is None or self._waiting: + return {} + return _read_port_mappings(self._handle) + def kill(self) -> None: """SIGKILL the child's entire process group by pid (like the sandbox and the Go binding), so a kill from another thread is safe while :meth:`wait` diff --git a/python/tests/test_popen.py b/python/tests/test_popen.py index 11273644..0283d87e 100644 --- a/python/tests/test_popen.py +++ b/python/tests/test_popen.py @@ -277,7 +277,8 @@ def test_sandbox_lifecycle_methods_reject_popen_process(): sandbox = _policy() proc = sandbox.popen(["sleep", "100"]) try: - for op in (sandbox.wait, sandbox.kill, sandbox.pause, sandbox.resume): + for op in (sandbox.wait, sandbox.kill, sandbox.pause, sandbox.resume, + sandbox.checkpoint): with pytest.raises(RuntimeError): op() # The Process still works after the rejected sandbox-level calls. @@ -287,6 +288,23 @@ def test_sandbox_lifecycle_methods_reject_popen_process(): proc.wait() +def test_sandbox_pid_and_ports_delegate_to_popen_process(): + # Sandbox.pid/ports must read the popen child through the Process (whose + # accessors are lock-guarded), not touch the Process's handle directly — + # otherwise a concurrent wait() free would race the raw FFI read. + sandbox = _policy() + proc = sandbox.popen(["sleep", "100"]) + try: + assert proc.pid is not None + assert sandbox.pid == proc.pid + assert sandbox.ports() == proc.ports() # both {} without port_remap + finally: + proc.kill() + proc.wait() + assert sandbox.pid is None + assert sandbox.ports() == {} + + def test_wait_timeout_zero_does_not_wait_forever(): # timeout=0 (and sub-millisecond values) must not collapse to "wait forever"; # they return promptly with a non-success (killed) result. Watchdog so a