mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
fix(install): clear stale Windows cua lock
This commit is contained in:
yuexiong
Teknium
parent
1a6754efa3
commit
83dc0b9b85
2 changed files with 199 additions and 17 deletions
|
|
@ -986,27 +986,116 @@ _CUA_INSTALLER_TIMEOUT = 660
|
|||
_CUA_LOCK_STALE_AFTER = 600
|
||||
|
||||
|
||||
def _cua_install_home() -> "Path":
|
||||
"""Package home shared by the upstream POSIX and Windows installers."""
|
||||
return Path(
|
||||
os.environ.get("CUA_DRIVER_RS_HOME")
|
||||
or str(Path.home() / ".cua-driver")
|
||||
)
|
||||
|
||||
|
||||
def _cua_install_lock_dir() -> "Path":
|
||||
"""Path of the upstream installer's concurrent-install lock dir."""
|
||||
home = os.environ.get("CUA_DRIVER_RS_HOME") or str(Path.home() / ".cua-driver")
|
||||
return Path(home) / "packages" / ".install.lock.d"
|
||||
return _cua_install_home() / "packages" / ".install.lock.d"
|
||||
|
||||
|
||||
def _cua_windows_install_lock_file() -> "Path":
|
||||
"""Path of install.ps1's FileShare::None lock file."""
|
||||
return _cua_install_home() / "install.lock"
|
||||
|
||||
|
||||
def _clear_stale_windows_cua_install_lock() -> None:
|
||||
"""Delete install.ps1's lock file only when no process still holds it.
|
||||
|
||||
``install.ps1`` serializes installs with a ``FileStream`` opened using
|
||||
``FileShare::None``. Mirror that primitive with a zero-share
|
||||
``CreateFileW`` probe. ``FILE_FLAG_DELETE_ON_CLOSE`` removes an unlocked
|
||||
leftover atomically when the probe handle closes, avoiding a gap where a
|
||||
new installer could acquire the file between our probe and deletion.
|
||||
"""
|
||||
lock_file = _cua_windows_install_lock_file()
|
||||
try:
|
||||
if not lock_file.is_file():
|
||||
return
|
||||
|
||||
import ctypes as _ctypes
|
||||
from ctypes import wintypes as _wintypes
|
||||
|
||||
# Win32 constants used by install.ps1's FileShare::None equivalent.
|
||||
delete_access = 0x00010000
|
||||
generic_read = 0x80000000
|
||||
generic_write = 0x40000000
|
||||
open_existing = 3
|
||||
file_attribute_normal = 0x00000080
|
||||
file_flag_delete_on_close = 0x04000000
|
||||
|
||||
kernel32 = _ctypes.WinDLL("kernel32", use_last_error=True)
|
||||
create_file = kernel32.CreateFileW
|
||||
create_file.argtypes = [
|
||||
_wintypes.LPCWSTR,
|
||||
_wintypes.DWORD,
|
||||
_wintypes.DWORD,
|
||||
_wintypes.LPVOID,
|
||||
_wintypes.DWORD,
|
||||
_wintypes.DWORD,
|
||||
_wintypes.HANDLE,
|
||||
]
|
||||
create_file.restype = _wintypes.HANDLE
|
||||
close_handle = kernel32.CloseHandle
|
||||
close_handle.argtypes = [_wintypes.HANDLE]
|
||||
close_handle.restype = _wintypes.BOOL
|
||||
|
||||
handle = create_file(
|
||||
str(lock_file),
|
||||
generic_read | generic_write | delete_access,
|
||||
0, # FileShare::None
|
||||
None,
|
||||
open_existing,
|
||||
file_attribute_normal | file_flag_delete_on_close,
|
||||
None,
|
||||
)
|
||||
invalid_handle = _wintypes.HANDLE(-1).value
|
||||
if handle == invalid_handle:
|
||||
logger.debug(
|
||||
"Windows cua install lock at %s is still held or cannot be "
|
||||
"removed (winerror %s)",
|
||||
lock_file,
|
||||
_ctypes.get_last_error(),
|
||||
)
|
||||
return
|
||||
|
||||
if not close_handle(handle):
|
||||
logger.debug(
|
||||
"could not close Windows cua install lock probe at %s "
|
||||
"(winerror %s)",
|
||||
lock_file,
|
||||
_ctypes.get_last_error(),
|
||||
)
|
||||
return
|
||||
if lock_file.exists():
|
||||
logger.debug(
|
||||
"Windows cua install lock probe succeeded but %s remains",
|
||||
lock_file,
|
||||
)
|
||||
return
|
||||
|
||||
logger.info("Cleared stale Windows cua-driver install lock at %s", lock_file)
|
||||
_print_info(f" Cleared stale cua-driver install lock ({lock_file}).")
|
||||
except Exception as e:
|
||||
logger.debug("stale Windows cua install lock check failed: %s", e)
|
||||
|
||||
|
||||
def _clear_stale_cua_install_lock() -> None:
|
||||
"""Best-effort: remove a stale installer lock left by a dead holder.
|
||||
|
||||
A previous timed-out/killed install can orphan
|
||||
``~/.cua-driver/packages/.install.lock.d`` (the holder's pid is stamped
|
||||
into its ``info`` file). The upstream installer only reclaims it after
|
||||
waiting 600s — longer than our old subprocess timeout — so an orphaned
|
||||
lock wedged every subsequent refresh. Clear it up front when the holder
|
||||
is provably dead; leave it alone when the holder is alive (a slow
|
||||
concurrent install) or liveness can't be determined.
|
||||
|
||||
POSIX-only: the lock protocol lives in the bash installer; install.ps1
|
||||
does not use it.
|
||||
The POSIX installer stamps its holder pid into
|
||||
``~/.cua-driver/packages/.install.lock.d/info``. The Windows installer
|
||||
instead holds ``~/.cua-driver/install.lock`` open with
|
||||
``FileShare::None``. Clear either artifact up front only when its
|
||||
platform-specific liveness check proves that no install still holds it.
|
||||
"""
|
||||
if sys.platform == "win32":
|
||||
_clear_stale_windows_cua_install_lock()
|
||||
return
|
||||
lock_dir = _cua_install_lock_dir()
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -21,8 +21,11 @@ cleanly on missing-arch assets, and the upgrade path uses
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
class TestInstallCuaDriverUpgrade:
|
||||
def test_upgrade_on_unsupported_platform_is_silent_noop(self):
|
||||
|
|
@ -329,7 +332,11 @@ class TestArchProbeRemoval:
|
|||
urlopen.assert_not_called()
|
||||
|
||||
|
||||
class TestStaleInstallLockClear:
|
||||
@pytest.mark.skipif(
|
||||
sys.platform == "win32",
|
||||
reason="POSIX installer uses the .install.lock.d directory protocol",
|
||||
)
|
||||
class TestPosixStaleInstallLockClear:
|
||||
"""_clear_stale_cua_install_lock: pre-clears the upstream installer's
|
||||
concurrent-install lock only when the holder is provably dead (or the
|
||||
lock is old and pid-less). Issue #58762."""
|
||||
|
|
@ -391,6 +398,87 @@ class TestStaleInstallLockClear:
|
|||
tools_config._clear_stale_cua_install_lock() # must not raise
|
||||
|
||||
|
||||
class TestWindowsStaleInstallLockClearDispatch:
|
||||
def test_windows_branch_uses_file_lock_probe(self):
|
||||
from hermes_cli import tools_config
|
||||
|
||||
with patch.object(tools_config.sys, "platform", "win32"), \
|
||||
patch.object(
|
||||
tools_config, "_clear_stale_windows_cua_install_lock"
|
||||
) as clear_windows:
|
||||
tools_config._clear_stale_cua_install_lock()
|
||||
|
||||
clear_windows.assert_called_once_with()
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
sys.platform != "win32",
|
||||
reason="requires native Win32 FileShare semantics",
|
||||
)
|
||||
class TestWindowsStaleInstallLockClear:
|
||||
def _make_lock(self, tmp_path):
|
||||
import os
|
||||
|
||||
home = tmp_path / ".cua-driver"
|
||||
home.mkdir()
|
||||
lock = home / "install.lock"
|
||||
lock.write_text("pid=stale\n", encoding="utf-8")
|
||||
os.environ["CUA_DRIVER_RS_HOME"] = str(home)
|
||||
return lock
|
||||
|
||||
def teardown_method(self):
|
||||
import os
|
||||
|
||||
os.environ.pop("CUA_DRIVER_RS_HOME", None)
|
||||
|
||||
def test_unlocked_lock_file_is_cleared(self, tmp_path):
|
||||
from hermes_cli import tools_config
|
||||
|
||||
lock = self._make_lock(tmp_path)
|
||||
with patch.object(tools_config, "_print_info"):
|
||||
tools_config._clear_stale_cua_install_lock()
|
||||
|
||||
assert not lock.exists()
|
||||
|
||||
def test_lock_held_with_file_share_none_is_kept(self, tmp_path):
|
||||
import ctypes
|
||||
from ctypes import wintypes
|
||||
from hermes_cli import tools_config
|
||||
|
||||
lock = self._make_lock(tmp_path)
|
||||
kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
|
||||
create_file = kernel32.CreateFileW
|
||||
create_file.argtypes = [
|
||||
wintypes.LPCWSTR,
|
||||
wintypes.DWORD,
|
||||
wintypes.DWORD,
|
||||
wintypes.LPVOID,
|
||||
wintypes.DWORD,
|
||||
wintypes.DWORD,
|
||||
wintypes.HANDLE,
|
||||
]
|
||||
create_file.restype = wintypes.HANDLE
|
||||
close_handle = kernel32.CloseHandle
|
||||
close_handle.argtypes = [wintypes.HANDLE]
|
||||
close_handle.restype = wintypes.BOOL
|
||||
handle = create_file(
|
||||
str(lock),
|
||||
0x80000000 | 0x40000000, # GENERIC_READ | GENERIC_WRITE
|
||||
0, # FileShare::None, matching install.ps1
|
||||
None,
|
||||
3, # OPEN_EXISTING
|
||||
0x00000080, # FILE_ATTRIBUTE_NORMAL
|
||||
None,
|
||||
)
|
||||
assert handle != wintypes.HANDLE(-1).value
|
||||
|
||||
try:
|
||||
tools_config._clear_stale_cua_install_lock()
|
||||
assert lock.exists()
|
||||
finally:
|
||||
assert close_handle(handle)
|
||||
|
||||
|
||||
class TestInstallerTimeoutKillsProcessGroup:
|
||||
"""On timeout the whole installer process group must be killed, so the
|
||||
`curl | bash` grandchildren can't survive holding the install lock."""
|
||||
|
|
@ -399,11 +487,11 @@ class TestInstallerTimeoutKillsProcessGroup:
|
|||
import os
|
||||
import signal
|
||||
import subprocess
|
||||
import sys as _sys
|
||||
from unittest.mock import MagicMock
|
||||
from hermes_cli import tools_config
|
||||
|
||||
killed = {}
|
||||
sigkill = getattr(signal, "SIGKILL", signal.SIGTERM)
|
||||
|
||||
fake_proc = MagicMock()
|
||||
fake_proc.pid = 12345
|
||||
|
|
@ -418,10 +506,15 @@ class TestInstallerTimeoutKillsProcessGroup:
|
|||
killed["sig"] = sig
|
||||
|
||||
with patch("platform.system", return_value="Linux"), \
|
||||
patch.object(signal, "SIGKILL", sigkill, create=True), \
|
||||
patch("subprocess.run", return_value=MagicMock(returncode=0, stderr="")), \
|
||||
patch("subprocess.Popen", return_value=fake_proc), \
|
||||
patch.object(tools_config.os, "getpgid", return_value=99999), \
|
||||
patch.object(tools_config.os, "killpg", side_effect=fake_killpg), \
|
||||
patch.object(
|
||||
tools_config.os, "getpgid", return_value=99999, create=True
|
||||
), \
|
||||
patch.object(
|
||||
tools_config.os, "killpg", side_effect=fake_killpg, create=True
|
||||
), \
|
||||
patch.object(tools_config, "_clear_stale_cua_install_lock"), \
|
||||
patch.object(tools_config, "_print_warning"), \
|
||||
patch.object(tools_config, "_print_info"):
|
||||
|
|
@ -429,7 +522,7 @@ class TestInstallerTimeoutKillsProcessGroup:
|
|||
|
||||
assert ok is False
|
||||
assert killed.get("pgid") == 99999
|
||||
assert killed.get("sig") == signal.SIGKILL
|
||||
assert killed.get("sig") == sigkill
|
||||
# Post-kill reap happened.
|
||||
assert fake_proc.communicate.call_count == 2
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue