mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
test(cli): drive the Windows pid probe through an injected kernel32
This commit is contained in:
parent
a4563db77a
commit
6c584de435
2 changed files with 135 additions and 18 deletions
|
|
@ -8,6 +8,7 @@ import subprocess
|
|||
import sys
|
||||
import threading
|
||||
import time
|
||||
from collections.abc import Callable
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
|
@ -143,19 +144,17 @@ _STILL_ACTIVE: Final = 259
|
|||
_ERROR_ACCESS_DENIED: Final = 5
|
||||
|
||||
|
||||
def _windows_pid_exists(pid: int) -> bool:
|
||||
"""Liveness check for Windows that does not signal the process.
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class _WindowsProcessApi:
|
||||
"""The kernel32 surface ``_windows_pid_exists`` needs, injected so the probe is testable off Windows."""
|
||||
|
||||
Opens a query-only handle and asks for the exit code. ``OpenProcess``
|
||||
failing with ``ERROR_ACCESS_DENIED`` means the pid exists but is not ours
|
||||
to open, which is the case the ``PermissionError`` arm covers on POSIX.
|
||||
open_query_handle: Callable[[int], int]
|
||||
exit_code: Callable[[int], int | None]
|
||||
close_handle: Callable[[int], None]
|
||||
last_error: Callable[[], int]
|
||||
|
||||
Caveat kept deliberately: a process whose real exit code is 259 reads as
|
||||
alive, because ``GetExitCodeProcess`` reports ``STILL_ACTIVE`` (259) for a
|
||||
running process and cannot distinguish the two. That is the standard
|
||||
trade-off for this API and it is strictly better than the previous
|
||||
behaviour, which killed the process it was asked about.
|
||||
"""
|
||||
|
||||
def _load_windows_process_api() -> _WindowsProcessApi: # pragma: no cover - kernel32 is Windows-only; CI is Linux
|
||||
import ctypes
|
||||
from ctypes import wintypes
|
||||
|
||||
|
|
@ -167,16 +166,43 @@ def _windows_pid_exists(pid: int) -> bool:
|
|||
kernel32.CloseHandle.argtypes = (wintypes.HANDLE,)
|
||||
kernel32.CloseHandle.restype = wintypes.BOOL
|
||||
|
||||
handle: Final = kernel32.OpenProcess(_PROCESS_QUERY_LIMITED_INFORMATION, False, pid)
|
||||
def exit_code(handle: int) -> int | None:
|
||||
code = wintypes.DWORD()
|
||||
if not kernel32.GetExitCodeProcess(handle, ctypes.byref(code)):
|
||||
return None
|
||||
return code.value
|
||||
|
||||
return _WindowsProcessApi(
|
||||
open_query_handle=lambda pid: kernel32.OpenProcess(_PROCESS_QUERY_LIMITED_INFORMATION, False, pid),
|
||||
exit_code=exit_code,
|
||||
close_handle=kernel32.CloseHandle,
|
||||
last_error=ctypes.get_last_error,
|
||||
)
|
||||
|
||||
|
||||
def _windows_pid_exists(pid: int, api: _WindowsProcessApi | None = None) -> bool:
|
||||
"""Liveness check for Windows that does not signal the process.
|
||||
|
||||
Opens a query-only handle and asks for the exit code. ``OpenProcess``
|
||||
failing with ``ERROR_ACCESS_DENIED`` means the pid exists but is not ours
|
||||
to open, which is the case the ``PermissionError`` arm covers on POSIX.
|
||||
An unreadable exit code is reported as alive rather than guessed away.
|
||||
|
||||
Caveat kept deliberately: a process whose real exit code is 259 reads as
|
||||
alive, because ``GetExitCodeProcess`` reports ``STILL_ACTIVE`` (259) for a
|
||||
running process and cannot distinguish the two. That is the standard
|
||||
trade-off for this API and it is strictly better than the previous
|
||||
behaviour, which killed the process it was asked about.
|
||||
"""
|
||||
resolved: Final = api if api is not None else _load_windows_process_api()
|
||||
handle: Final = resolved.open_query_handle(pid)
|
||||
if not handle:
|
||||
return ctypes.get_last_error() == _ERROR_ACCESS_DENIED
|
||||
return resolved.last_error() == _ERROR_ACCESS_DENIED
|
||||
try:
|
||||
exit_code: Final = wintypes.DWORD()
|
||||
if not kernel32.GetExitCodeProcess(handle, ctypes.byref(exit_code)):
|
||||
return True
|
||||
return exit_code.value == _STILL_ACTIVE
|
||||
code: Final = resolved.exit_code(handle)
|
||||
return code is None or code == _STILL_ACTIVE
|
||||
finally:
|
||||
kernel32.CloseHandle(handle)
|
||||
resolved.close_handle(handle)
|
||||
|
||||
|
||||
def is_running(pid: int) -> bool:
|
||||
|
|
|
|||
|
|
@ -14,6 +14,8 @@ from litellm.proxy.client.cli.commands.autoroute.process import (
|
|||
PidRecord,
|
||||
ProcessLaunchError,
|
||||
UpError,
|
||||
_windows_pid_exists,
|
||||
_WindowsProcessApi,
|
||||
clear_pid_record,
|
||||
is_port_available,
|
||||
is_running,
|
||||
|
|
@ -44,6 +46,18 @@ def _signals_sent(fake_kill: Mock) -> tuple[int, ...]:
|
|||
return tuple(call.args[1] for call in fake_kill.call_args_list)
|
||||
|
||||
|
||||
def _windows_api(handle: int, exit_code: int | None = None, last_error: int = 0) -> tuple[_WindowsProcessApi, Mock]:
|
||||
"""A kernel32 stand-in, plus the Mock that recorded ``CloseHandle``."""
|
||||
close_handle: Final = Mock()
|
||||
api: Final = _WindowsProcessApi(
|
||||
open_query_handle=Mock(return_value=handle),
|
||||
exit_code=Mock(return_value=exit_code),
|
||||
close_handle=close_handle,
|
||||
last_error=Mock(return_value=last_error),
|
||||
)
|
||||
return api, close_handle
|
||||
|
||||
|
||||
class TestIsPortAvailable:
|
||||
def test_true_for_a_free_port(self):
|
||||
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
|
||||
|
|
@ -185,6 +199,83 @@ class TestIsRunningDoesNotSignal:
|
|||
assert _signals_sent(fake_kill) == (), "os.kill reached on win32; signal 0 is CTRL_C_EVENT there"
|
||||
|
||||
|
||||
class TestWindowsPidExists:
|
||||
"""The Windows probe, driven through an injected kernel32 so it runs on any platform.
|
||||
|
||||
``OpenProcess`` / ``GetExitCodeProcess`` only exist on Windows, so CI could
|
||||
never execute this decision logic before. The four calls the probe needs
|
||||
now arrive as a ``_WindowsProcessApi``; only the loader that builds the real
|
||||
one out of ``ctypes`` stays Windows-only.
|
||||
"""
|
||||
|
||||
_ERROR_ACCESS_DENIED: Final = 5
|
||||
_ERROR_INVALID_PARAMETER: Final = 87
|
||||
_STILL_ACTIVE: Final = 259
|
||||
|
||||
def test_access_denied_on_open_means_the_pid_exists(self):
|
||||
"""A pid we may not open is still a live pid; POSIX answers the same way via PermissionError."""
|
||||
api, close_handle = _windows_api(handle=0, last_error=self._ERROR_ACCESS_DENIED)
|
||||
|
||||
assert _windows_pid_exists(4321, api) is True
|
||||
close_handle.assert_not_called()
|
||||
|
||||
def test_any_other_open_failure_means_the_pid_is_gone(self):
|
||||
api, close_handle = _windows_api(handle=0, last_error=self._ERROR_INVALID_PARAMETER)
|
||||
|
||||
assert _windows_pid_exists(4321, api) is False
|
||||
close_handle.assert_not_called()
|
||||
|
||||
def test_still_active_means_running(self):
|
||||
api, close_handle = _windows_api(handle=99, exit_code=self._STILL_ACTIVE)
|
||||
|
||||
assert _windows_pid_exists(4321, api) is True
|
||||
close_handle.assert_called_once_with(99)
|
||||
|
||||
def test_a_real_exit_code_means_the_process_finished(self):
|
||||
api, close_handle = _windows_api(handle=99, exit_code=0)
|
||||
|
||||
assert _windows_pid_exists(4321, api) is False
|
||||
close_handle.assert_called_once_with(99)
|
||||
|
||||
def test_an_unreadable_exit_code_is_reported_as_running(self):
|
||||
"""``GetExitCodeProcess`` failing is not evidence the process died, so do not claim it did."""
|
||||
api, close_handle = _windows_api(handle=99, exit_code=None)
|
||||
|
||||
assert _windows_pid_exists(4321, api) is True
|
||||
close_handle.assert_called_once_with(99)
|
||||
|
||||
def test_the_handle_is_released_even_when_the_query_raises(self):
|
||||
"""A leaked query handle would keep the exited process object alive for the life of the CLI."""
|
||||
close_handle: Final = Mock()
|
||||
api: Final = _WindowsProcessApi(
|
||||
open_query_handle=Mock(return_value=99),
|
||||
exit_code=Mock(side_effect=OSError(22, "Invalid argument")),
|
||||
close_handle=close_handle,
|
||||
last_error=Mock(return_value=0),
|
||||
)
|
||||
|
||||
with pytest.raises(OSError):
|
||||
_windows_pid_exists(4321, api)
|
||||
|
||||
close_handle.assert_called_once_with(99)
|
||||
|
||||
def test_the_probe_asks_about_the_pid_it_was_given(self):
|
||||
api, _ = _windows_api(handle=99, exit_code=self._STILL_ACTIVE)
|
||||
|
||||
_windows_pid_exists(4321, api)
|
||||
|
||||
api.open_query_handle.assert_called_once_with(4321)
|
||||
|
||||
def test_the_probe_never_signals_the_process(self, monkeypatch):
|
||||
"""The bug this replaces: ``os.kill(pid, 0)`` is ``CTRL_C_EVENT`` on Windows."""
|
||||
fake_kill: Final = Mock()
|
||||
monkeypatch.setattr(process_module.os, "kill", fake_kill)
|
||||
api, _ = _windows_api(handle=99, exit_code=self._STILL_ACTIVE)
|
||||
|
||||
assert _windows_pid_exists(4321, api) is True
|
||||
assert _signals_sent(fake_kill) == ()
|
||||
|
||||
|
||||
class TestTerminateHardKillSignal:
|
||||
def test_escalation_resolves_a_signal_when_sigkill_is_absent(self, monkeypatch):
|
||||
"""``signal.SIGKILL`` does not exist on Windows.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue