fix(update): the Node build's job launcher runs on the base interpreter, and a refused join degrades
A venv's Scripts\python.exe is a redirector that starts the base interpreter as its own child inside a job of its own. That child cannot join the update job once the job holds the updater's git children (AssignProcessToJobObject: ERROR_ACCESS_DENIED, 'could not join the update job: 5', every install.ps1 TUI build on the Windows runners), and the redirector keeps the inherited job handle open, so the job never closed on the owner's death (test_killed_owner_takes_its_node_build_down). The stdlib launcher now runs on sys._base_executable, and a refused join runs the command outside the job with a stderr notice instead of failing the build. The stable-zip identity test injects its git faults at update_custody.run as well: on Windows inside an update that seam is a suspended Popen bound to the job, not subprocess.run.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user