fix(windows): trampoline hermes update out of its own launcher - #86101
fix(windows): trampoline hermes update out of its own launcher#86101adamcap926 wants to merge 1 commit into
Conversation
On Windows `hermes update` can never complete. `hermes` is launched
through a distlib console-script launcher, which is not a thin redirect:
it spawns `venv\Scripts\python.exe` as a child, hands it the launcher's
own path as the script, and blocks. Every invocation is two processes:
hermes.exe (pid A, holds Scripts\hermes.exe as its running image)
\_ python.exe (pid B, runs hermes_cli.main)
The update ends in `uv pip install -e .`, which rewrites the console-
script shims -- including the one pid A has mapped. Windows refuses to
replace a running image, so uv fails with "The process cannot access the
file because it is being used by another process. (os error 32)". The
git path fails, the ZIP fallback fails identically, and no retry can
succeed: nothing pid B does releases the lock, because the holder is its
parent.
Remove the hazard rather than race it. Before the update touches
anything, re-launch it as a detached `python -m hermes_cli.main`
grandchild and let pids B and A exit. The grandchild waits for the
launcher to disappear, then updates against an ordinary unlocked file:
hermes.exe (A) --> python.exe (B) --> python.exe (C, detached)
exit exit waits for A, then updates
Hooked into cmd_update after --check (which installs nothing) and before
the update lock is acquired: a trampolining parent that had already
written the lock marker would release it moments later while the
grandchild ran unlocked, since the grandchild's acquire() sees its
still-live parent as an ancestor and would run under a claim about to
vanish.
stdin is DEVNULL so the detached updater cannot race the reclaimed shell
for keystrokes. That also selects the existing non-interactive path, so
local changes follow updates.non_interactive_local_changes (default
stash) rather than prompting where nobody can answer.
Off Windows maybe_trampoline() returns False immediately and behavior is
unchanged; POSIX replaces a running executable's inode atomically.
Verified on Windows 11 / Python 3.11.15 / uv, non-elevated: before, four
consecutive failed updates; after, the launcher exits, the grandchild
completes the update, and no os error 32 occurs on either path.
Scoped deliberately to the trampoline. The residual case where another
process holds a shim, and cleanup of the reboot-deferred rename entries
that case queues, are addressed by NousResearch#68821 and NousResearch#85942 respectively and
are not duplicated here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGLv7LyNrpaPm4ciGqhyxD
3a6fa7a to
c7b26a8
Compare
|
Narrowed this PR after searching for overlapping work. It was three fixes in one; two of them duplicated open PRs, so I've dropped those and left only the part nothing else covers. Now: 2 files, +357/-0 -- What I dropped and why
What's left, and why it isn't covered elsewhereThe CLI self-lock itself. #85679 applies the same insight ("drive the hand-off through the venv python, not the hermes.exe shim") but only in Worth noting the three are complementary rather than competing: this removes the cause, #68821 handles the residual case where a genuinely foreign process holds a shim, and #85942 cleans up what earlier versions already queued on affected machines. All three are needed for a machine that has already hit this. The previous, wider version of this branch is preserved locally if any of the dropped material is wanted here after all. |
|
Superseded by #86326, which takes the same off-the-shim approach and adds a source-level regression guard. Credited you as co-author there. Thanks for the fix and the thorough write-up. |
|
Validated on a real Windows 11 host. The trampoline works end-to-end under a Validation performed:
On the Complement, not duplicate, of #85679: this fixes the CLI path One edge note from testing (not a bug in this PR): Happy to run further scenarios on the Windows host if useful. |
|
Thanks for the credit on #86326, and @Halldrix thank you for the validation run \u2014 that was a real Windows host doing a real shim rewrite, which is not a cheap thing to set up. One scope note before this closes for good, and I may be misreading it, so please correct me if so. #86326 changes Checking current So on current main the CLI path still has both halves of the original problem:
Repro on Windows, unelevated, no desktop app involved: with anything that forces uv to actually rewrite the shim. On the machine this came from, one failed run queued four I am not asking to reverse the merge \u2014 #86326 is a good fix for the path it covers. The question is just whether the CLI path is considered covered by it. If it isn't, I'm happy to either have this reopened or to file a fresh PR against current |
The bug
On Windows,
hermes updatecan never complete. It fails with the same error on the git path, the ZIP fallback, and every retry:hermesis launched through a distlib console-script launcher. That launcher is not a thin redirect -- it spawnsvenv\Scripts\python.exeas a child, hands it the launcher's own path as the script, and blocks. Every invocation is two processes:hermes updateends inuv pip install -e ., which rewrites the console-script shims -- including the one pid A has mapped. Windows refuses. Nothing pid B can do releases the lock, because the holder is its parent.Why the existing mitigation could not work
_quarantine_running_hermes_exerenames the shim aside first, which is the right idea. But when the rename lost, it fell back toMoveFileExW(MOVEFILE_DELAY_UNTIL_REBOOT)and thencontinued as though the path were free.It is not. That API moves nothing at call time -- it only appends the rename to
PendingFileRenameOperationsfor next boot. The shim stays exactly where it was, so the install fails identically, the git path "fails", the ZIP fallback runs and fails the same way, and each attempt queues another registry entry.The user-facing message
The new shim was written at the same pathis not true; nothing was written.The part that bites later
Because no replacement shim is ever written, applying those queued entries at boot renames
hermes.exeaway and leaves nothing behind. A failed update silently removeshermesfrom PATH on the user's next restart, with nothing on screen connecting it to an update that failed days earlier.On the machine this was found on, one failed update had queued four such entries.
The fix
1. Trampoline out of the launcher (new
hermes_cli/win_self_update.py)Before the update touches anything, re-launch it as a detached
python -m hermes_cli.maingrandchild and let pids B and A exit. The grandchild waits for the launcher to disappear, then updates against an ordinary unlocked file.Hooked into
cmd_updatebefore the update lock is acquired -- a trampolining parent that had already written the marker would release it moments later while the grandchild ran unlocked. Placed after--check, which installs nothing.stdinisDEVNULLso the detached updater cannot race the reclaimed shell for keystrokes; that also selects the existing non-interactive path, so local changes followupdates.non_interactive_local_changes(defaultstash) instead of asking a question nobody can answer.2. Stop treating a reboot-deferred rename as success
_quarantine_running_hermes_exenow raisesShimQuarantineError._run_quarantined_installrolls back shims it already moved, and the user gets a remedy instead of a traceback ending in an opaque uv error.3. Detect the landmine already queued on affected machines
pending_shim_renames()readsPendingFileRenameOperationsand warns before each update.tools/clear-pending-shim-renames.ps1clears the entries pairwise, preserving unrelated pending operations -- the usual advice of clearing the whole key breaks Windows Update, Edge, and AV products.Verification
Reproduced and fixed on Windows 11, Python 3.11.15, uv, non-elevated.
Before: four consecutive failures, four queued registry entries,
hermes.exedue to be orphaned at next boot.After, running
hermes updatethrough the shim:Launcher exits immediately, the grandchild completes the update, zero
os error 32, and no newPendingFileRenameOperationsentries queued.Non-Windows is unaffected:
maybe_trampolinereturnsFalseimmediately off Windows, and POSIX replaces a running executable's inode atomically.Note for maintainers
One adjacent issue surfaced during testing and is not addressed here: the gateway's supervisor respawns it inside the pause-to-guard window, so
_detect_venv_python_processestrips on a gateway the updater itself just paused, and the update refuses until the user runshermes gateway stopmanually. It fails safe, so I left it alone rather than widen this PR.Generated with Claude Code
https://claude.ai/code/session_01FGLv7LyNrpaPm4ciGqhyxD