From 83dc0b9b85bb8c86ec4aeb012d406e3a4086b6e1 Mon Sep 17 00:00:00 2001 From: yuexiong Date: Sun, 26 Jul 2026 08:58:30 +0800 Subject: [PATCH] fix(install): clear stale Windows cua lock --- hermes_cli/tools_config.py | 113 +++++++++++++++++--- tests/hermes_cli/test_install_cua_driver.py | 103 +++++++++++++++++- 2 files changed, 199 insertions(+), 17 deletions(-) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 6139f7e0acaf..1d3f6899fa9a 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -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: diff --git a/tests/hermes_cli/test_install_cua_driver.py b/tests/hermes_cli/test_install_cua_driver.py index 73b95b2daac8..bd532ac7fb81 100644 --- a/tests/hermes_cli/test_install_cua_driver.py +++ b/tests/hermes_cli/test_install_cua_driver.py @@ -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