diff --git a/hermes_cli/update_custody.py b/hermes_cli/update_custody.py index 1160f2f83c..41f8cf435d 100644 --- a/hermes_cli/update_custody.py +++ b/hermes_cli/update_custody.py @@ -77,8 +77,10 @@ def git_argv(git_cmd: Sequence[str], args: Sequence[str]) -> list[str]: def _held() -> dict | None: - from hermes_cli import update_lock - + try: + from hermes_cli import update_lock + except Exception: # noqa: BLE001 - a torn tree's launch repair: no updater runs from it + return None return update_lock._HELD @@ -196,20 +198,43 @@ def run_git(git_cmd: Sequence[str], args: Sequence[str], **kwargs) -> subprocess # A stdlib launcher for children whose Popen the updater never sees (``run_contained``): it # joins the job named by an inherited handle, drops that handle (only the owner's handle may -# keep the job open) and runs the real command with the same stdio. +# keep the job open) and runs the real command with the same stdio. A refused join never fails +# the build: the command runs outside the job (the pre-custody behavior) and says so on stderr. +_CUSTODY_UNAVAILABLE = "hermes: update custody unavailable" _JOIN_JOB = ( "import ctypes, subprocess, sys\n" "k = ctypes.WinDLL('kernel32', use_last_error=True)\n" "k.AssignProcessToJobObject.argtypes = [ctypes.c_void_p, ctypes.c_void_p]\n" "k.GetCurrentProcess.restype = ctypes.c_void_p\n" + "k.CloseHandle.argtypes = [ctypes.c_void_p]\n" "h = ctypes.c_void_p(int(sys.argv[1]))\n" "if not k.AssignProcessToJobObject(h, k.GetCurrentProcess()):\n" - " sys.exit('could not join the update job: %d' % ctypes.get_last_error())\n" + f" sys.stderr.write('{_CUSTODY_UNAVAILABLE} (could not join the update job: %d); '\n" + " 'this child runs outside it\\n' % ctypes.get_last_error())\n" + " sys.stderr.flush()\n" "k.CloseHandle(h)\n" "sys.exit(subprocess.call(sys.argv[2:], stdin=subprocess.DEVNULL))\n" ) +def _join_launcher_python() -> str: + """The interpreter for the job-joining launcher: the real one, never a venv redirector. + + A venv's ``Scripts\\python.exe`` is a redirector: it starts the base interpreter as its own + child, inside a job of its own. That child cannot join the update's job once the job holds a + process from another job hierarchy (the updater's git children): ``AssignProcessToJobObject`` + fails with ERROR_ACCESS_DENIED (5). And the redirector, never joined, keeps its inherited copy + of the job handle open, so the job would not close when the owner dies. The launcher is + stdlib-only (``-I -S``), so the base interpreter runs it as is. + """ + import os + + base = getattr(sys, "_base_executable", None) + if base and os.path.isfile(base): + return base + return sys.executable + + @contextlib.contextmanager def contained_command(argv: Sequence[str], *, inherit_lock: bool = True): """``(argv, kwargs)`` for a checkout writer started by a runner that hides its Popen (the Node @@ -227,7 +252,7 @@ def contained_command(argv: Sequence[str], *, inherit_lock: bool = True): try: info = subprocess.STARTUPINFO() info.lpAttributeList = {"handle_list": [handle]} - yield [sys.executable, "-I", "-S", "-c", _JOIN_JOB, str(handle), *argv], {"startupinfo": info} + yield [_join_launcher_python(), "-I", "-S", "-c", _JOIN_JOB, str(handle), *argv], {"startupinfo": info} finally: import ctypes diff --git a/tests/hermes_cli/test_update_lock_windows_live.py b/tests/hermes_cli/test_update_lock_windows_live.py index bce79602d1..36c2a4e324 100644 --- a/tests/hermes_cli/test_update_lock_windows_live.py +++ b/tests/hermes_cli/test_update_lock_windows_live.py @@ -89,8 +89,12 @@ from pathlib import Path sys.path.insert(0, sys.argv[1]) from hermes_cli.update_lock import UpdateLock from hermes_cli.source_build import run_source_script +from hermes_cli.update_custody import run_git root = Path(sys.argv[2]) assert UpdateLock(path=Path(sys.argv[3]), install_root=root).acquire() +# A git child first, as in every install/update: the job then already holds a process when the +# build's launcher joins it (a venv redirector's child was refused here: ERROR_ACCESS_DENIED). +run_git(["git"], ["--version"], capture_output=True, check=True) env = {**os.environ, "PATH": sys.argv[4] + os.pathsep + os.environ["PATH"]} run_source_script(root, "build.mjs", env=env, label="probe build") """ @@ -172,3 +176,15 @@ def test_killed_owner_takes_its_node_build_down(tmp_path): (bin_dir / "node.bat").write_text(f'@"{sys.executable}" "{install / "build.py"}"\r\n', encoding="utf-8") _tree_dies_with_its_owner(tmp_path, install, [_BUILD_OWNER, str(REPO_ROOT), str(install), str(tmp_path / "m"), str(bin_dir)]) + + +def test_a_refused_job_join_still_runs_the_child(tmp_path): + """Custody degrades, never fails: a launcher whose join is refused runs its command anyway.""" + from hermes_cli.update_custody import _CUSTODY_UNAVAILABLE, _JOIN_JOB + + child = "import sys; print('built'); sys.exit(3)" + out = subprocess.run([sys.executable, "-I", "-S", "-c", _JOIN_JOB, "0", sys.executable, "-c", child], + capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=60) + assert out.returncode == 3, out + assert out.stdout.strip() == "built" + assert _CUSTODY_UNAVAILABLE in out.stderr diff --git a/tests/hermes_cli/test_update_target_identity.py b/tests/hermes_cli/test_update_target_identity.py index 2533f6658a..318b178f68 100644 --- a/tests/hermes_cli/test_update_target_identity.py +++ b/tests/hermes_cli/test_update_target_identity.py @@ -324,7 +324,11 @@ def test_stable_zip_consumes_the_same_commit_through_the_real_swap(update_tree, local += '?' + parsed.query return real_open(local, *args, **kwargs) - def guarded_run(command, *args, **kwargs): + from hermes_cli import update_custody + + real_custody_run, seam = update_custody.run, threading.local() + + def guard(command, args, kwargs, call): nonlocal fetched, failed command = list(map(str, command)) assert Path(command[0]).name.lower() in {'git', 'git.exe'}, command @@ -337,11 +341,28 @@ def test_stable_zip_consumes_the_same_commit_through_the_real_swap(update_tree, if fetched and not failed and '--abbrev-ref' in command and kwargs.get('check'): failed = True raise subprocess.CalledProcessError(128, command, '', 'fixture: Git file I/O failed') - result = real_run(command, *args, **kwargs) + result = call(command, *args, **kwargs) if 'fetch' in command: fetched = True return result + def guarded_run(command, *args, **kwargs): + if getattr(seam, 'custody', False): # already guarded at the custody seam below + return real_run(command, *args, **kwargs) + return guard(command, args, kwargs, real_run) + + def custody_call(command, *args, **kwargs): + seam.custody = True + try: + return real_custody_run(command, *args, **kwargs) + finally: + seam.custody = False + + def guarded_custody_run(command, *args, **kwargs): + # The updater's git goes through update_custody.run: on Windows inside an update that is a + # suspended Popen bound to the update's job, not subprocess.run, so faults go in here too. + return guard(command, args, kwargs, custody_call) + if transport in {'gitless', 'no-git'}: (t.clone / '.git').rename(tmp_path / 'git-state') if transport == 'dirty': @@ -349,6 +370,7 @@ def test_stable_zip_consumes_the_same_commit_through_the_real_swap(update_tree, before = (t.clone / 'content.txt').read_bytes() monkeypatch.setattr(urllib.request, 'urlopen', local_open) monkeypatch.setattr(subprocess, 'run', guarded_run) + monkeypatch.setattr(update_custody, 'run', guarded_custody_run) try: if transport == 'dirty': with pytest.raises(SystemExit) as error: