diff --git a/docs/system-specs/common/platform-compat.md b/docs/system-specs/common/platform-compat.md index 071c2a87822..98aa01c1bbe 100644 --- a/docs/system-specs/common/platform-compat.md +++ b/docs/system-specs/common/platform-compat.md @@ -31,6 +31,7 @@ produces exactly those silent failures, which is why the helper is named per cal | Parent PID | `get_ppid(pid)` | `/proc` read / libproc | | Match process cmdline | `process_matches(pid, needles)` | `/proc//cmdline` / `ps` | | Process start time (PID-reuse guard) | `process_start_time(pid)` | `/proc//stat` / `ps -o lstart=` (both answer `None` on Windows, so the guard silently never confirms) | +| Is this pid a PROCESS rather than a thread | `is_thread_group_leader(pid)` for one pid; `live_thread_group_leaders()` once for a whole sweep | `pid_exists(pid)` alone (Linux numbers threads from the pid space and POSIX permits signalling a tid, so a pid recycled as a THREAD of an unrelated process reads as alive forever). Both answer `None`, never `False`, when unknowable — treat `None` as "retain", never as licence to act | | Signals | `platform_compat.SIGKILL` / `SIGTERM` | `signal.SIGKILL` (undefined on Windows) | | Spawn isolation | `start_new_session=IS_POSIX` + `creationflags=CREATE_NEW_PROCESS_GROUP` | bare `start_new_session=True` | | Re-exec the current Python module | `reexec_python_module(module, args)` | `os.execv(sys.executable, [sys.executable, ...])` (breaks when the Windows interpreter path contains spaces) | diff --git a/docs/system-specs/modules/session.md b/docs/system-specs/modules/session.md index 751cf09ed6c..0b9d6811ea4 100644 --- a/docs/system-specs/modules/session.md +++ b/docs/system-specs/modules/session.md @@ -1467,7 +1467,30 @@ a trust root on its own; publication therefore also writes a sidecar; strict resolvers fail closed until the next turn's publish re-signs the mapping. Benign and self-healing — no migration step. - **Stale cleanup**: the orphan sweep removes `session_pid_.sig` - alongside its `.txt` for dead pids (`session_pid.py`). + alongside its `.txt` for dead pids (`session_pid.py`). "Dead" is not + `pid_exists` alone: Linux numbers threads from the pid space, so a dead + session's pid recycled as a THREAD of an unrelated live process still + satisfies that probe and the mapping would survive forever (observed on a + host whose pid counter had wrapped: 233 mappings, one naming a 6-day-dead + session through a thread). `_prune_stale_session_pid_files` therefore + removes a mapping when the pid is unsignalable, OR when it is absent from + one `platform_compat.live_thread_group_leaders()` snapshot **and** a + per-pid `platform_compat.is_thread_group_leader(pid)` re-read returns + `False`. Both helpers answer `None` when the question is unknowable + (non-Linux, unreadable `/proc`), and `None` never licenses a removal — so + macOS and Windows keep the pre-existing `pid_exists`-only behaviour. Two + orderings are load-bearing: the snapshot is taken AFTER the glob (a pid + that starts in that window lands IN the set and is retained), and absence + from it selects a *candidate* rather than the outcome, so a pid recycled + since the snapshot — whose new owner has already republished the mapping at + that same path — is retained by the re-read instead of losing a live + session's identity. The snapshot costs one `/proc` directory read for the + whole pass, which is still work the gateway boot path does not carry: + `narrow_with_leaders=False` there per `no-new-work-on-gateway-boot-path` + (and on the force-exit handler, which must reach its `os._exit`), while the + graceful-shutdown sweep asks for the narrowing. This pass touches only the + `session_pid_` family, never the shared `kiro_session_pids.txt` that + pass 1 rewrites. - **Threat model** (full version in the `session_pid_sig.py` module docstring): file forgery, cross-pid replay, tampering, and symlink planting are blocked; deliberate same-uid impersonation via diff --git a/src/kiro_crew/platform_compat.py b/src/kiro_crew/platform_compat.py index bf4905a96df..d5cb9c1c75a 100644 --- a/src/kiro_crew/platform_compat.py +++ b/src/kiro_crew/platform_compat.py @@ -2814,6 +2814,79 @@ def process_owner_uid(pid: int) -> int | None: PID_UNSIGNALABLE = "unsignalable" # exists but we cannot signal it (POSIX EPERM) +def live_thread_group_leaders() -> frozenset[int] | None: + """Every pid on the host that is a PROCESS, or ``None`` when unknowable. + + Linux numbers threads from the same space as processes and exposes + ``/proc/`` for them, and POSIX permits signalling a tid — so a tid + satisfies both :func:`pid_exists` and :func:`pid_liveness` while naming no + process at all. A caller holding a recorded pid therefore cannot tell "my + process is still alive" from "that number now belongs to some unrelated + process's thread", which matters once the pid counter wraps + (``/proc/sys/kernel/pid_max`` is commonly 4194304 and a busy host cycles it + in hours). + + The discriminator is ``/proc`` itself: its top-level listing enumerates + ONLY thread-group leaders. A non-leader tid is absent from that listing + even though ``/proc/`` stays directly openable — which is exactly why + the cheaper per-pid probes cannot see the difference. + + Deliberately ONE directory read for the whole host rather than a read per + pid. A sweep over N recorded mappings costs a single ``os.listdir`` instead + of N opens of ``/proc//status`` (measured on Linux: 1.5 ms once versus + 7.8 ms across 233 mappings), so no per-entry synchronous file read happens + on the caller's thread at all. Also cheaper than ``process_matches``, which + shells out to ``ps`` on macOS and so cannot be used per entry in a sweep. + + Returns ``None`` — never an empty set — whenever the answer is not knowable + (non-Linux, unreadable ``/proc``, or a listing with no numeric entries). + Callers use this to *narrow* a liveness check, so an inconclusive result + must never be the thing that decides a pid is stale: treat ``None`` as + "retain everything". + """ + if not IS_LINUX: + return None + try: + entries = os.listdir("/proc") + except OSError: + return None + leaders = {int(name) for name in entries if name.isdigit()} + if not leaders: + return None + return frozenset(leaders) + + +def is_thread_group_leader(pid: int) -> bool | None: + """Whether ``pid`` names a PROCESS right now, or ``None`` when unknowable. + + The per-pid counterpart to :func:`live_thread_group_leaders`, for the one + question a host-wide snapshot cannot answer. A snapshot is a reading taken at + an instant, so a pid recycled AFTER it was taken is absent from it while + naming a live process. A caller about to act destructively on "absent from + the snapshot" therefore needs a reading taken now, for that pid alone. + + ``/proc//status`` carries ``Tgid``, the pid of the thread group's + leader, so ``Tgid == pid`` is a process while a non-leader tid reports its + leader's pid instead. One file read is the cheap way to ask about one pid, + where the top-level ``/proc`` listing is the cheap way to ask about all of + them -- which is why this narrows the snapshot rather than replacing it. + + Returns ``None`` -- never ``False`` -- whenever the answer is not knowable + (non-Linux, the pid is gone, an unreadable or malformed ``status``), so an + inconclusive read can never be the thing that licenses a destructive action. + """ + if not IS_LINUX: + return None + try: + with open(f"/proc/{pid}/status", encoding="utf-8", errors="replace") as handle: + for line in handle: + if line.startswith("Tgid:"): + return int(line.split()[1]) == pid + except (OSError, ValueError, IndexError): + return None + return None + + def pid_liveness(pid: int) -> str: """Three-way liveness probe: PID_DEAD / PID_ALIVE / PID_UNSIGNALABLE. diff --git a/src/kiro_crew/session_pid.py b/src/kiro_crew/session_pid.py index 02dc069868e..da2a56dd215 100644 --- a/src/kiro_crew/session_pid.py +++ b/src/kiro_crew/session_pid.py @@ -709,7 +709,7 @@ def _cleanup_orphaned_mcp_servers() -> int: return killed -def cleanup_orphaned_sessions() -> None: +def cleanup_orphaned_sessions(*, narrow_with_leaders: bool = True) -> None: """Kill leftover kiro-cli processes from a previous gateway run. Reads ``kiro_session_pids.txt`` (written at spawn time), validates each @@ -722,6 +722,13 @@ def cleanup_orphaned_sessions() -> None: Also sweeps orphaned MCP server processes via ``_cleanup_orphaned_mcp_servers`` which uses the separate ``kiro_pids.txt`` (child:parent format). + ``narrow_with_leaders`` is forwarded to + :func:`_prune_stale_session_pid_files`. The gateway passes ``False`` on its + boot path and in its force-exit handler, so both do exactly the work they + did before the recycled-pid change; the narrowing applies on the graceful + shutdown path, which is not spawning sessions. + + Additionally cleans up: - Stale ``session_pid_*.txt`` files for processes that no longer exist. - Empty directories under ``sessions/`` left by subagents that produced @@ -759,8 +766,50 @@ def _skip_tagged(gw_pid: int, _pid: int) -> bool: logger.info("Cleaned up %d orphaned MCP server processes", mcp_killed) # Third pass: remove stale session_pid_*.txt files for dead processes + _prune_stale_session_pid_files(narrow_with_leaders=narrow_with_leaders) + + # Fourth pass: remove empty session workspace dirs (orphaned subagent dirs) + sessions_dir = config_dir() / "sessions" + empty_dirs = 0 + if sessions_dir.exists(): + for d in sessions_dir.iterdir(): + if d.is_dir() and not any(d.iterdir()): + try: + d.rmdir() + empty_dirs += 1 + except OSError: + pass # directory became non-empty or was already removed + if empty_dirs: + logger.info("Cleaned up %d empty session workspace dirs", empty_dirs) + + +def _prune_stale_session_pid_files(*, narrow_with_leaders: bool = True) -> int: + """Remove ``session_pid_.txt`` mappings whose pid is not that session. + + ``narrow_with_leaders`` decides whether the thread-group-leaders snapshot is + taken. It costs one ``/proc`` directory read for the whole pass and is what + catches a pid recycled as a THREAD of a live process, but it is work the + gateway boot path may not carry: ``no-new-work-on-gateway-boot-path`` names + orphan sweeps specifically, so the boot caller passes ``False``. The + narrowing is asked for on the graceful shutdown path instead. + + A live session's pid is both signalable and a thread-group leader, so it is + retained under either setting, and this pass never touches the shared + ``kiro_session_pids.txt`` that pass 1 rewrites. + + Returns the number of mapping files removed. + """ stale_pid_files = 0 - for pid_file in config_dir().glob("session_pid_*.txt"): + pid_files = list(config_dir().glob("session_pid_*.txt")) + # Snapshot the host's thread-group leaders ONCE for the whole sweep — one + # directory read instead of a synchronous /proc read per mapping. + # + # Ordering matters: snapshot AFTER globbing. A pid that starts in the window + # between the two lands IN the set and is retained; one that exits in that + # window is absent and is pruned, which is correct. Snapshotting first would + # invert both. + leaders = platform_compat.live_thread_group_leaders() if narrow_with_leaders else None + for pid_file in pid_files: try: pid = int(pid_file.stem.removeprefix("session_pid_")) except ValueError: @@ -773,29 +822,45 @@ def _skip_tagged(gw_pid: int, _pid: int) -> bool: logger.debug("Could not remove malformed pid file: %s", pid_file.name) continue # os.kill(pid, 0) would terminate the process on Windows — probe instead. - if not platform_compat.pid_exists(pid): - pid_file.unlink(missing_ok=True) - # Remove the HMAC sidecar (session_pid_.sig) alongside its - # .txt — a dangling sidecar is harmless (verification requires - # both) but would accumulate forever. - pid_file.with_suffix(".sig").unlink(missing_ok=True) - stale_pid_files += 1 + # + # The leaders set narrows the liveness test: a dead session's pid can be + # recycled as a THREAD of an unrelated live process, and a tid satisfies + # ``pid_exists``, so that probe alone would keep the mapping forever. + # + # Resolution of a TOKEN-BEARING mapping is already safe without this: + # ``session_pid_sig._pid_recycled`` compares the live start token and + # refuses on a mismatch on both the strict and the lenient path, and a + # tid's live start token cannot match the dead process's. What this + # sweep adds is (a) pruning LEGACY token-less mappings, where that + # guard has no recorded token to compare and callers keep resolving, + # and (b) bounding accumulation — observed on a host whose pid counter + # had wrapped: 233 mappings, 1 still naming a 6-day-dead session via a + # thread of an unrelated process. + # + # ``leaders is None`` means the question was unanswerable (non-Linux, + # unreadable /proc), so it never contributes to a prune. + if platform_compat.pid_exists(pid): + if leaders is None or pid in leaders: + continue + # Absence from the snapshot selects a CANDIDATE, never the outcome. + # The snapshot was read before this loop, so a pid recycled since -- + # whose mapping the new owner has already republished at this same + # path -- is missing from it while naming a LIVE session. Unlinking + # that mapping would lose a live session's identity, so the decision + # needs a reading for this pid taken now. Retain on anything but a + # definite "not a process", and pay the per-pid read only for the + # few candidates rather than for every mapping. + if platform_compat.is_thread_group_leader(pid) is not False: + continue + pid_file.unlink(missing_ok=True) + # Remove the HMAC sidecar (session_pid_.sig) alongside its + # .txt — a dangling sidecar is harmless (verification requires + # both) but would accumulate forever. + pid_file.with_suffix(".sig").unlink(missing_ok=True) + stale_pid_files += 1 if stale_pid_files: logger.info("Cleaned up %d stale session PID files", stale_pid_files) - - # Fourth pass: remove empty session workspace dirs (orphaned subagent dirs) - sessions_dir = config_dir() / "sessions" - empty_dirs = 0 - if sessions_dir.exists(): - for d in sessions_dir.iterdir(): - if d.is_dir() and not any(d.iterdir()): - try: - d.rmdir() - empty_dirs += 1 - except OSError: - pass # directory became non-empty or was already removed - if empty_dirs: - logger.info("Cleaned up %d empty session workspace dirs", empty_dirs) + return stale_pid_files def cleanup_orphaned_session_roots() -> int: diff --git a/src/kiro_crew/slack/gateway.py b/src/kiro_crew/slack/gateway.py index dd206cb9604..cc97875736e 100644 --- a/src/kiro_crew/slack/gateway.py +++ b/src/kiro_crew/slack/gateway.py @@ -11215,7 +11215,19 @@ def _probe_persistence() -> str | None: # Clean up orphaned kiro-cli processes from previous runs from kiro_crew.session import cleanup_orphaned_sessions - cleanup_orphaned_sessions() + # Off-loop: the sweep is synchronous filesystem work and this runs + # inside the orchestrator coroutine. + # + # ``narrow_with_leaders=False`` keeps boot doing exactly what it did + # before the recycled-pid work: the leaders snapshot is a /proc read this + # path may not carry, since no-new-work-on-gateway-boot-path names orphan + # sweeps. + # + # The narrowing is asked for on the graceful-shutdown sweep only, so a + # gateway that is hard-killed never runs it and its recycled-pid mappings + # wait for a later clean exit. That is the accepted cost of keeping this + # path, and the force-exit handler, doing exactly their pre-existing work. + await asyncio.to_thread(cleanup_orphaned_sessions, narrow_with_leaders=False) # Same "previous run left residue" concern as the orphan sweep above, for # telemetry rather than processes: any open-session crumb on disk belongs @@ -11469,7 +11481,16 @@ def _on_signal(*_args: object) -> None: nonlocal _shutting_down if _shutting_down: print("\n👻 Force exit!") - cleanup_orphaned_sessions() + # Synchronous by necessity: a signal handler cannot await. + # The process calls os._exit immediately below, so loop latency + # does not matter on this path. + # + # ``narrow_with_leaders=False`` so this handler does exactly the + # work it did before the recycled-pid change: killing leftover + # processes is what this path is for, and a handler that reaches + # for extra work before its os._exit is a handler that may not + # get there. + cleanup_orphaned_sessions(narrow_with_leaders=False) # os._exit skips atexit, so the log queue's drain hook never # runs — flush the queued gateway.log tail here, bounded so a # wedged disk cannot hang the force exit. @@ -11700,8 +11721,13 @@ async def _start_bg_session() -> None: logger.warning("Graceful shutdown timed out — force exiting") print("👻 Goodbye!") - # Kill any kiro-cli processes that survived graceful shutdown - cleanup_orphaned_sessions() + # Kill any kiro-cli processes that survived graceful shutdown. + # Off-loop: still inside the orchestrator coroutine here. + # + # The one call site that asks for the leaders narrowing. Nothing spawns a + # session by this point, so the sweep is not racing a mapping publisher -- + # the same position the sweep already held here before this change. + await asyncio.to_thread(cleanup_orphaned_sessions) # This is a hard exit too: os._exit skips atexit, so the log queue's # drain hook never runs here either. Without this the whole shutdown # tail is lost -- including the "Graceful shutdown timed out" warning diff --git a/test/test_pid_lifecycle.py b/test/test_pid_lifecycle.py index 1f107bd82aa..3ccafcf85e0 100644 --- a/test/test_pid_lifecycle.py +++ b/test/test_pid_lifecycle.py @@ -7,6 +7,7 @@ import signal import subprocess import sys +import threading from collections import deque from collections.abc import Iterator from pathlib import Path @@ -366,6 +367,145 @@ def unlink_that_fails_on_bad(path_self, *a, **kw): assert (tmp_path / "session_pid_bad!name.txt").exists() assert not (tmp_path / "session_pid_99999.txt").exists() + @pytest.mark.skipif(sys.platform != "linux", reason="tids share the pid space on Linux only") + def test_pid_file_recycled_as_a_thread_is_deleted( + self, tmp_path: Path, session_pid_file: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A mapping whose pid now names a THREAD of a live process is stale. + + Linux draws tids from the pid space and lets you signal one, so such a + pid passes ``pid_exists`` and such a mapping would survive forever. A + token-bearing mapping is already safe to resolve — ``_pid_recycled`` + refuses on a start-token mismatch, and a tid's token cannot match — so + what is pruned here is the legacy token-less form, which has no recorded + token for that guard to compare, plus the accumulation itself. + + Uses a real live thread's native tid rather than a fake ``/proc``, so + the test exercises the same kernel behaviour that produced the bug. + """ + from kiro_crew.session_pid import cleanup_orphaned_sessions + + monkeypatch.setattr("kiro_crew.session_pid.config_dir", lambda: tmp_path) + session_pid_file.write_text("") + + tid_box: dict[str, int] = {} + release = threading.Event() + captured = threading.Event() + + def _hold() -> None: + tid_box["tid"] = threading.get_native_id() + captured.set() + release.wait(timeout=30) + + holder = threading.Thread(target=_hold, daemon=True) + holder.start() + assert captured.wait(timeout=30), "helper thread never reported its tid" + tid = tid_box["tid"] + assert tid != os.getpid(), "native_id must differ from the group leader" + + try: + thread_map = tmp_path / f"session_pid_{tid}.txt" + leader_map = tmp_path / f"session_pid_{os.getpid()}.txt" + thread_map.write_text("sess-recycled-as-thread") + leader_map.write_text("sess-live-leader") + + # NOT patching os.kill: both pids are genuinely signalable here, + # which is exactly the condition the old predicate could not split. + with patch("kiro_crew.session_pid._cleanup_orphaned_mcp_servers", return_value=0): + cleanup_orphaned_sessions() + + assert not thread_map.exists(), "a pid that is only a thread must be pruned" + assert leader_map.exists(), "a live thread-group leader must be retained" + finally: + release.set() + holder.join(timeout=30) + + def test_boot_setting_reads_no_proc( + self, tmp_path: Path, session_pid_file: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """``narrow_with_leaders=False`` must not take the leaders snapshot. + + The gateway boot path passes it so the sweep costs exactly what it cost + before this branch: ``no-new-work-on-gateway-boot-path`` names orphan + sweeps, so a regression that read the leaders set anyway would put a + ``/proc`` scan back on the boot path, where the readiness cost of it is + not visible to anyone reading the sweep. + """ + from kiro_crew.session_pid import cleanup_orphaned_sessions + + monkeypatch.setattr("kiro_crew.session_pid.config_dir", lambda: tmp_path) + session_pid_file.write_text("") + + def _refuse() -> set[int] | None: + raise AssertionError("the boot setting must not read /proc for leaders") + + monkeypatch.setattr( + "kiro_crew.session_pid.platform_compat.live_thread_group_leaders", _refuse + ) + with patch("kiro_crew.session_pid._cleanup_orphaned_mcp_servers", return_value=0): + cleanup_orphaned_sessions(narrow_with_leaders=False) + + def test_prune_pass_leaves_the_shared_pid_file_alone( + self, tmp_path: Path, session_pid_file: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The deferred pass must not rewrite the file the boot sweep owns. + + Deferring the prune past readiness is only sound because this pass touches + ``session_pid_.txt`` mappings and nothing else. If it also rewrote + ``kiro_session_pids.txt`` it would race the spawns that append to it once + the gateway is serving, and a lost entry is an unkillable orphan. + """ + from kiro_crew.session_pid import _prune_stale_session_pid_files + + monkeypatch.setattr("kiro_crew.session_pid.config_dir", lambda: tmp_path) + session_pid_file.write_text("111:222\n") + (tmp_path / "session_pid_99999.txt").write_text("sess-dead") + + # The probe is pinned, not assumed: ``pid_max`` is 4194304 here, so 99999 + # is an ordinary live pid on a host whose counter has passed it, and a live + # pid is retained -- which would fail the removal assertion below on a + # long-running runner rather than in review. The sibling sweeps above pin + # it the same way; ``os.kill`` is what ``platform_compat.pid_exists`` + # reaches for on POSIX. + with patch("os.kill", side_effect=ProcessLookupError): + removed = _prune_stale_session_pid_files() + + assert removed == 1 + assert not (tmp_path / "session_pid_99999.txt").exists() + assert session_pid_file.read_text() == "111:222\n" + + def test_stale_snapshot_does_not_delete_a_live_mapping( + self, tmp_path: Path, session_pid_file: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A pid recycled after the snapshot keeps the mapping its new owner wrote. + + The leaders snapshot is read once for the whole pass, so a pid that became + a live process after it was taken is absent from it while naming a LIVE + session whose mapping already sits at that path. Deciding on the snapshot + alone unlinks that live mapping, which is a lost session identity, not a + tidy-up; the per-pid re-read is what refuses. The shipped call sites do not + run this pass beside live sessions, so this is defence in depth rather than + load-bearing -- it keeps the guarantee a property of the function instead of + of where it happens to be called from. + """ + from kiro_crew.session_pid import _prune_stale_session_pid_files + + monkeypatch.setattr("kiro_crew.session_pid.config_dir", lambda: tmp_path) + live = tmp_path / f"session_pid_{os.getpid()}.txt" + live.write_text("sess-published-after-the-snapshot") + + # A snapshot from before this process existed: the pid is signalable and + # is a real thread-group leader, yet absent from the set. + monkeypatch.setattr( + "kiro_crew.session_pid.platform_compat.live_thread_group_leaders", + lambda: frozenset({1}), + ) + + removed = _prune_stale_session_pid_files() + + assert removed == 0 + assert live.exists(), "a live leader absent from a stale snapshot must be retained" + class TestResetStateUntracksParentPid: def test_reset_state_untracks_parent_pid(self) -> None: diff --git a/test/test_platform_compat.py b/test/test_platform_compat.py index 7e4409a4af2..23763a09373 100644 --- a/test/test_platform_compat.py +++ b/test/test_platform_compat.py @@ -4565,3 +4565,122 @@ def test_no_lock_site_opens_truncating(self): "lock files opened truncating before the acquire (GH-9248); " "use platform_compat.open_lock_file: " + ", ".join(offenders) ) + + +class TestLiveThreadGroupLeaders: + """``live_thread_group_leaders`` narrows liveness; it must fail OPEN.""" + + @pytest.mark.skipif(sys.platform != "linux", reason="/proc lists leaders on Linux only") + def test_own_process_is_a_leader(self): + leaders = pc.live_thread_group_leaders() + assert leaders is not None + assert os.getpid() in leaders + + @pytest.mark.skipif(sys.platform != "linux", reason="tids share the pid space on Linux only") + def test_a_live_thread_is_not_a_leader(self): + """The whole point: a tid is signalable and has /proc, but is not a process. + + Uses a real thread's native id rather than a synthetic ``/proc`` so the + assertion rests on kernel behaviour, not on a fixture's idea of it. + """ + box: dict[str, int] = {} + captured = threading.Event() + release = threading.Event() + + def _hold() -> None: + box["tid"] = threading.get_native_id() + captured.set() + release.wait(timeout=30) + + holder = threading.Thread(target=_hold, daemon=True) + holder.start() + try: + assert captured.wait(timeout=30) + tid = box["tid"] + assert tid != os.getpid() + # Signalable and openable under /proc — the two things a naive check reads. + assert pc.pid_exists(tid) is True + leaders = pc.live_thread_group_leaders() + assert leaders is not None + assert tid not in leaders, "a non-leader tid must not appear in the /proc listing" + assert os.getpid() in leaders, "its group leader must still appear" + finally: + release.set() + holder.join(timeout=30) + + def test_non_linux_fails_open(self, monkeypatch): + """Off Linux the question is unanswerable, so never claim 'thread'.""" + monkeypatch.setattr(pc, "IS_LINUX", False) + assert pc.live_thread_group_leaders() is None + + def test_unreadable_proc_fails_open(self, monkeypatch): + """An OSError reading /proc yields None (retain), never an empty set.""" + monkeypatch.setattr(pc, "IS_LINUX", True) + + def _boom(*_args, **_kwargs): + raise OSError("permission denied") + + monkeypatch.setattr(pc.os, "listdir", _boom) + assert pc.live_thread_group_leaders() is None + + def test_numberless_proc_fails_open(self, monkeypatch): + """A listing with no pids is nonsense, not 'every recorded pid is a thread'.""" + monkeypatch.setattr(pc, "IS_LINUX", True) + monkeypatch.setattr(pc.os, "listdir", lambda *_a, **_k: ["cpuinfo", "meminfo", "self"]) + assert pc.live_thread_group_leaders() is None + + +class TestIsThreadGroupLeader: + """The per-pid re-read, for a pid a host-wide snapshot answers wrongly.""" + + @pytest.mark.skipif(sys.platform != "linux", reason="/proc carries Tgid on Linux only") + def test_own_process_is_a_leader(self): + assert pc.is_thread_group_leader(os.getpid()) is True + + @pytest.mark.skipif(sys.platform != "linux", reason="tids share the pid space on Linux only") + def test_a_live_thread_is_not_a_leader(self): + """The whole point: a signalable tid must answer False, not None.""" + box: dict[str, int] = {} + captured = threading.Event() + release = threading.Event() + + def _hold() -> None: + box["tid"] = threading.get_native_id() + captured.set() + release.wait(timeout=30) + + holder = threading.Thread(target=_hold, daemon=True) + holder.start() + try: + assert captured.wait(timeout=30), "helper thread never reported its tid" + tid = box["tid"] + assert tid != os.getpid() + assert pc.pid_exists(tid) is True, "a tid is signalable -- that is the trap" + assert pc.is_thread_group_leader(tid) is False + finally: + release.set() + holder.join(timeout=30) + + def test_non_linux_is_unknowable(self, monkeypatch): + monkeypatch.setattr(pc, "IS_LINUX", False) + assert pc.is_thread_group_leader(os.getpid()) is None + + def test_missing_status_is_unknowable(self, monkeypatch): + """A pid that has gone must not read as 'not a process'.""" + monkeypatch.setattr(pc, "IS_LINUX", True) + + def _boom(*_a, **_k): + raise FileNotFoundError("no such pid") + + monkeypatch.setattr("builtins.open", _boom) + assert pc.is_thread_group_leader(4242) is None + + def test_malformed_status_is_unknowable(self, monkeypatch): + """A status file with no parsable Tgid answers None, never False.""" + import io + + monkeypatch.setattr(pc, "IS_LINUX", True) + monkeypatch.setattr( + "builtins.open", lambda *_a, **_k: io.StringIO("Name:\tx\nTgid:\tnotanumber\n") + ) + assert pc.is_thread_group_leader(4242) is None