mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-11-06 05:02:24 +01:00
fix(browser): probe pid liveness through the platform-safe helper
_process_is_alive used os.kill(pid, 0). That probe is POSIX-only: CPython's Windows os.kill calls TerminateProcess(handle, sig) for any signal other than CTRL_C/CTRL_BREAK, so it terminates the process it is asked about. This function is only reached when there is no procfs to read a command line from, which is exactly the macOS and Windows case the rest of this change exists to handle. core/platform_compat.py already owns that probe and documents the hazard; its module docstring asks callers to import from there rather than spell a POSIX-only call out locally. Delegate to it. pid_alive answers False where os.kill raises PermissionError — a live process owned by another user. Both call sites want that reading: the sweep only unlinks a pid file it wrote itself, and a pid it cannot confirm is not the daemon it is looking for.
This commit is contained in:
@@ -18,6 +18,7 @@ import urllib.request
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Dict, Any
|
from typing import Dict, Any
|
||||||
|
|
||||||
|
from core.platform_compat import pid_alive
|
||||||
from src.constants import MAX_OUTPUT_CHARS
|
from src.constants import MAX_OUTPUT_CHARS
|
||||||
|
|
||||||
PDF_EXTRACT_MAX_BYTES = 80_000_000
|
PDF_EXTRACT_MAX_BYTES = 80_000_000
|
||||||
@@ -143,18 +144,23 @@ def _process_command_line(pid: int) -> str | None:
|
|||||||
|
|
||||||
|
|
||||||
def _process_is_alive(pid: int) -> bool:
|
def _process_is_alive(pid: int) -> bool:
|
||||||
"""Whether a pid currently exists. Signal 0 checks without delivering."""
|
"""Whether a pid currently exists.
|
||||||
|
|
||||||
try:
|
Delegates to ``core.platform_compat.pid_alive`` rather than probing with
|
||||||
os.kill(pid, 0)
|
``os.kill(pid, 0)`` directly. That probe is POSIX-only: CPython's Windows
|
||||||
except ProcessLookupError:
|
``os.kill`` calls ``TerminateProcess(handle, sig)`` for any signal other
|
||||||
return False
|
than CTRL_C / CTRL_BREAK, so it would *kill* the daemon it is asked about.
|
||||||
except PermissionError:
|
Windows is also where there is no procfs, which is precisely when this
|
||||||
# Alive, owned by somebody else.
|
function gets called at all.
|
||||||
return True
|
|
||||||
except OSError:
|
``pid_alive`` reads False for a pid that ``os.kill`` reports with
|
||||||
return False
|
``PermissionError`` — a live process owned by another user. Neither caller
|
||||||
return True
|
here wants a different answer: the sweep only unlinks a pid file it wrote
|
||||||
|
itself, and treating somebody else's pid as "not our daemon" is the safe
|
||||||
|
reading in both.
|
||||||
|
"""
|
||||||
|
|
||||||
|
return pid_alive(pid)
|
||||||
|
|
||||||
_SCHOLARLY_METADATA_CUE_RE = re.compile(
|
_SCHOLARLY_METADATA_CUE_RE = re.compile(
|
||||||
r"\b(?:accept(?:ed|ance)?|publish(?:ed|ing|cation)?|venue|conference|"
|
r"\b(?:accept(?:ed|ance)?|publish(?:ed|ing|cation)?|venue|conference|"
|
||||||
|
|||||||
@@ -1923,3 +1923,27 @@ def test_procfs_host_still_matches_on_the_command_line(monkeypatch, tmp_path) ->
|
|||||||
|
|
||||||
_pid_file_for(tmp_path, monkeypatch, "clawmm-test", "session-6", 6666)
|
_pid_file_for(tmp_path, monkeypatch, "clawmm-test", "session-6", 6666)
|
||||||
assert PrivateBrowserTool._owned_daemon_exists({}, "session-6") is False
|
assert PrivateBrowserTool._owned_daemon_exists({}, "session-6") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_liveness_probe_goes_through_the_platform_safe_helper(monkeypatch) -> None:
|
||||||
|
"""The no-procfs path must not reach a bare ``os.kill(pid, 0)``.
|
||||||
|
|
||||||
|
CPython's Windows ``os.kill`` calls ``TerminateProcess(handle, sig)`` for
|
||||||
|
any signal other than CTRL_C / CTRL_BREAK, so probing liveness with signal
|
||||||
|
0 terminates the process it asks about — and the only hosts that reach this
|
||||||
|
probe are the ones with no procfs, Windows among them.
|
||||||
|
``core.platform_compat.pid_alive`` is the tree's platform-safe answer.
|
||||||
|
"""
|
||||||
|
|
||||||
|
asked: list[int] = []
|
||||||
|
monkeypatch.setattr(
|
||||||
|
web_tools, "pid_alive", lambda pid: asked.append(pid) or True
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
web_tools.os,
|
||||||
|
"kill",
|
||||||
|
lambda *a, **kw: pytest.fail("os.kill must not be used to probe liveness"),
|
||||||
|
)
|
||||||
|
|
||||||
|
assert web_tools._process_is_alive(4242) is True
|
||||||
|
assert asked == [4242]
|
||||||
|
|||||||
Reference in New Issue
Block a user