Skip to content
Open
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
31 changes: 29 additions & 2 deletions code_review_graph/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,10 @@ def clear_pid(path: Path | None = None) -> None:

# Win32 constants for the OpenProcess-based liveness check (#511).
_PROCESS_QUERY_LIMITED_INFORMATION = 0x1000
_SYNCHRONIZE = 0x00100000
_ERROR_ACCESS_DENIED = 5
_WAIT_OBJECT_0 = 0x0
_WAIT_FAILED = 0xFFFFFFFF


def _pid_alive_windows(
Expand All @@ -327,21 +329,36 @@ def _pid_alive_windows(
) -> bool:
"""Win32 PID liveness check via OpenProcess/WaitForSingleObject.

The access mask must include SYNCHRONIZE: a handle opened with only
PROCESS_QUERY_LIMITED_INFORMATION cannot be waited on, so
WaitForSingleObject returns WAIT_FAILED (ERROR_ACCESS_DENIED) and
every exited process reads as alive.

The kernel32 interface is injected so tests can drive handle/wait
outcomes on non-Windows platforms. *get_last_error* defaults to
``kernel32.GetLastError``; the real caller passes
``ctypes.get_last_error`` (reliable with ``use_last_error=True``).
"""
if get_last_error is None:
get_last_error = kernel32.GetLastError
handle = kernel32.OpenProcess(_PROCESS_QUERY_LIMITED_INFORMATION, False, pid)
handle = kernel32.OpenProcess(_PROCESS_QUERY_LIMITED_INFORMATION | _SYNCHRONIZE, False, pid)
if not handle:
# NULL handle: process is dead, or we lack access. ACCESS_DENIED
# means it exists but is owned by another user — treat as alive.
return get_last_error() == _ERROR_ACCESS_DENIED
try:
result = kernel32.WaitForSingleObject(handle, 0)
if result == _WAIT_FAILED:
# The wait itself errored — we cannot prove the process dead,
# so err alive, consistent with the ACCESS_DENIED branch.
logger.debug(
"WaitForSingleObject on PID %d failed (error %d); presuming alive",
pid,
get_last_error(),
)
return True
# WAIT_OBJECT_0 means the process handle is signaled (it exited).
return kernel32.WaitForSingleObject(handle, 0) != _WAIT_OBJECT_0
return result != _WAIT_OBJECT_0
finally:
kernel32.CloseHandle(handle)

Expand All @@ -355,8 +372,18 @@ def pid_alive(pid: int) -> bool:
"""
if sys.platform == "win32":
import ctypes
from ctypes import wintypes

kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
# Explicit prototypes: ctypes otherwise defaults every argument and
# return value to c_int, which truncates 64-bit HANDLEs and returns
# WAIT_FAILED as -1 — never equal to the unsigned 0xFFFFFFFF constant.
kernel32.OpenProcess.argtypes = (wintypes.DWORD, wintypes.BOOL, wintypes.DWORD)
kernel32.OpenProcess.restype = wintypes.HANDLE
kernel32.WaitForSingleObject.argtypes = (wintypes.HANDLE, wintypes.DWORD)
kernel32.WaitForSingleObject.restype = wintypes.DWORD
kernel32.CloseHandle.argtypes = (wintypes.HANDLE,)
kernel32.CloseHandle.restype = wintypes.BOOL
return _pid_alive_windows(pid, kernel32, ctypes.get_last_error)
try:
os.kill(pid, 0) # signal 0 = existence check
Expand Down
17 changes: 15 additions & 2 deletions tests/test_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,8 @@ def test_alive_when_wait_times_out(self):

kernel32 = _FakeKernel32(handle=1234, wait_result=0x102)
assert _pid_alive_windows(4242, kernel32) is True
# PROCESS_QUERY_LIMITED_INFORMATION, no inherit, the pid
assert kernel32.open_calls == [(0x1000, False, 4242)]
# PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE, no inherit, the pid
assert kernel32.open_calls == [(0x1000 | 0x00100000, False, 4242)]
assert kernel32.wait_calls == [(1234, 0)]
# The handle must always be closed
assert kernel32.closed == [1234]
Expand All @@ -393,6 +393,19 @@ def test_dead_when_handle_is_signaled(self):
assert _pid_alive_windows(4242, kernel32) is False
assert kernel32.closed == [1234]

def test_alive_when_wait_fails(self):
"""WAIT_FAILED (0xFFFFFFFF) cannot prove death — presume alive.

Regression guard: with a QUERY-only access mask the wait always
failed, so dead daemons were reported as running forever.
"""
from code_review_graph.daemon import _pid_alive_windows

kernel32 = _FakeKernel32(handle=1234, wait_result=0xFFFFFFFF)
assert _pid_alive_windows(4242, kernel32) is True
# The handle must still be closed on this path
assert kernel32.closed == [1234]

def test_alive_on_access_denied(self):
"""NULL handle + ERROR_ACCESS_DENIED (5) means alive (other user)."""
from code_review_graph.daemon import _pid_alive_windows
Expand Down