fix(env-probe): stuck Windows probe can no longer deadlock system-prompt builds - #67999
Merged
Conversation
…mpt builds An orphaned pip descendant holding the probe's inherited stdout/stderr pipe handles wedged subprocess.run's post-timeout communicate() (which joins the pipe reader threads with NO timeout on Windows). The warm probe thread then hung holding the module-level _CACHE_LOCK, so every new session's prompt build blocked indefinitely (#67964). Two layers: - _run(): replace subprocess.run with Popen + communicate(timeout); on TimeoutExpired kill the process TREE (taskkill /T on Windows) and reap the direct child bounded — never re-read the pipes, so an orphaned descendant holding them open can't block us. - get_environment_probe_line(): the probe now runs in a single background worker publishing via a threading.Event; callers wait at most _PROBE_WAIT_TIMEOUT (10s) then fail open with "". After one full timeout, later callers only peek. If the stuck worker ever finishes, its line resumes appearing in new prompts. Regression tests: hung probe with 4 concurrent callers returns bounded; late recovery publishes; repeat callers skip the wait; _run() returns promptly despite a pipe-holding descendant (real subprocess E2E). Fixes #67964
Collaborator
Related to open #67991 and #67964. Both address the same Windows env-probe deadlock; this PR additionally uses process-tree cancellation plus a bounded shared worker/fail-open path, so maintainers should choose or consolidate rather than treat it as a duplicate. |
teknium1
added a commit
that referenced
this pull request
Jul 23, 2026
…run + add no-window tests (#67690 follow-up) Follow-up to the #67690 salvage (@m4r13y). The PR's tools/env_probe.py hunk was written against the old capture_output=True _run(); #67964/#67999 rewrote _run to temp-file capture on July 20, so that hunk no longer applied — but the rewritten _run still lacked creationflags and kept flashing one console per probe (~5 per kanban worker start) from windowless parents. Re-implement the one-line fix against the current shape: creationflags=windows_hide_flags() on the temp-file subprocess.run, preserving the #67964 grandchild-can't-wedge-the-pipe contract. Also add the tests the PR didn't ship, in tests/test_windows_subprocess_no_window_flags.py: - env_probe._run passes CREATE_NO_WINDOW and keeps temp-file (non-PIPE) stdout/stderr + DEVNULL stdin - lazy_deps uv install / pip --version probe / pip install fallback / ensurepip bootstrap all pass CREATE_NO_WINDOW - suppress_platform_ver_console: POSIX no-op (platform._syscmd_ver untouched, win32_ver() still returns), and simulated-Windows stubbing (echo stub installed, idempotent, never raises)
teknium1
added a commit
that referenced
this pull request
Jul 24, 2026
…run + add no-window tests (#67690 follow-up) Follow-up to the #67690 salvage (@m4r13y). The PR's tools/env_probe.py hunk was written against the old capture_output=True _run(); #67964/#67999 rewrote _run to temp-file capture on July 20, so that hunk no longer applied — but the rewritten _run still lacked creationflags and kept flashing one console per probe (~5 per kanban worker start) from windowless parents. Re-implement the one-line fix against the current shape: creationflags=windows_hide_flags() on the temp-file subprocess.run, preserving the #67964 grandchild-can't-wedge-the-pipe contract. Also add the tests the PR didn't ship, in tests/test_windows_subprocess_no_window_flags.py: - env_probe._run passes CREATE_NO_WINDOW and keeps temp-file (non-PIPE) stdout/stderr + DEVNULL stdin - lazy_deps uv install / pip --version probe / pip install fallback / ensurepip bootstrap all pass CREATE_NO_WINDOW - suppress_platform_ver_console: POSIX no-op (platform._syscmd_ver untouched, win32_ver() still returns), and simulated-Windows stubbing (echo stub installed, idempotent, never raises)
teknium1
added a commit
that referenced
this pull request
Jul 24, 2026
…run + add no-window tests (#67690 follow-up) Follow-up to the #67690 salvage (@m4r13y). The PR's tools/env_probe.py hunk was written against the old capture_output=True _run(); #67964/#67999 rewrote _run to temp-file capture on July 20, so that hunk no longer applied — but the rewritten _run still lacked creationflags and kept flashing one console per probe (~5 per kanban worker start) from windowless parents. Re-implement the one-line fix against the current shape: creationflags=windows_hide_flags() on the temp-file subprocess.run, preserving the #67964 grandchild-can't-wedge-the-pipe contract. Also add the tests the PR didn't ship, in tests/test_windows_subprocess_no_window_flags.py: - env_probe._run passes CREATE_NO_WINDOW and keeps temp-file (non-PIPE) stdout/stderr + DEVNULL stdin - lazy_deps uv install / pip --version probe / pip install fallback / ensurepip bootstrap all pass CREATE_NO_WINDOW - suppress_platform_ver_console: POSIX no-op (platform._syscmd_ver untouched, win32_ver() still returns), and simulated-Windows stubbing (echo stub installed, idempotent, never raises)
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…mpt builds (NousResearch#67999) An orphaned pip descendant holding the probe's inherited stdout/stderr pipe handles wedged subprocess.run's post-timeout communicate() (which joins the pipe reader threads with NO timeout on Windows). The warm probe thread then hung holding the module-level _CACHE_LOCK, so every new session's prompt build blocked indefinitely (NousResearch#67964). Two layers: - _run(): replace subprocess.run with Popen + communicate(timeout); on TimeoutExpired kill the process TREE (taskkill /T on Windows) and reap the direct child bounded — never re-read the pipes, so an orphaned descendant holding them open can't block us. - get_environment_probe_line(): the probe now runs in a single background worker publishing via a threading.Event; callers wait at most _PROBE_WAIT_TIMEOUT (10s) then fail open with "". After one full timeout, later callers only peek. If the stuck worker ever finishes, its line resumes appearing in new prompts. Regression tests: hung probe with 4 concurrent callers returns bounded; late recovery publishes; repeat callers skip the wait; _run() returns promptly despite a pipe-holding descendant (real subprocess E2E). Fixes NousResearch#67964
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…run + add no-window tests (NousResearch#67690 follow-up) Follow-up to the NousResearch#67690 salvage (@m4r13y). The PR's tools/env_probe.py hunk was written against the old capture_output=True _run(); NousResearch#67964/NousResearch#67999 rewrote _run to temp-file capture on July 20, so that hunk no longer applied — but the rewritten _run still lacked creationflags and kept flashing one console per probe (~5 per kanban worker start) from windowless parents. Re-implement the one-line fix against the current shape: creationflags=windows_hide_flags() on the temp-file subprocess.run, preserving the NousResearch#67964 grandchild-can't-wedge-the-pipe contract. Also add the tests the PR didn't ship, in tests/test_windows_subprocess_no_window_flags.py: - env_probe._run passes CREATE_NO_WINDOW and keeps temp-file (non-PIPE) stdout/stderr + DEVNULL stdin - lazy_deps uv install / pip --version probe / pip install fallback / ensurepip bootstrap all pass CREATE_NO_WINDOW - suppress_platform_ver_console: POSIX no-op (platform._syscmd_ver untouched, win32_ver() still returns), and simulated-Windows stubbing (echo stub installed, idempotent, never raises)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A stuck local-toolchain probe can no longer deadlock system-prompt construction — a wedged
pip --versionsubprocess (Windows orphaned-descendant pipe hang, #67964) now degrades only the probe line itself, never new sessions.Root cause: on Windows,
subprocess.run(capture_output=True, timeout=3)reacts toTimeoutExpiredby callingcommunicate()a second time with no timeout to collect partial output. That call joins the pipe reader threads unboundedly. When the probedpip.exelauncher left an orphanedpython.exedescendant holding the inherited stdout/stderr write handles, the pipes never hit EOF, the warm-probe thread hung insidesubprocess._communicatewhile holding the module-level_CACHE_LOCK, and every new session's prompt build blocked atget_environment_probe_line()— zero API calls,workingforever (28 min in the reported incident until manual orphan kill).Changes
tools/env_probe.py::_run—Popen+communicate(timeout)instead ofsubprocess.run; on timeout: kill the process tree (taskkill /Ton Windows), boundedwait(1)reap, and never a second pipe read. Bounded always; abandoned daemon reader threads are capped (≤6/process, result cached).tools/env_probe.py::get_environment_probe_line— the probe now runs in a single background worker that publishes viathreading.Event. Callers wait at most 10s and fail open with""(prompt just omits the toolchain line). After one full timeout, later callers only peek (~50ms) — a permanently stuck probe costs the wait once, not per-session. If the stuck worker ever finishes, its line resumes appearing in new prompts (matches the incident: killing the orphan un-wedged everything with no restart).warm_environment_probe_async— same single-worker start path; a generation counter keeps test resets from racing stale workers.tests/tools/test_env_probe.py— regression tests: hung probe + 4 concurrent callers return bounded and fail open; late recovery publishes the line; repeat callers skip the full wait;_run()returns promptly against a real pipe-holding descendant subprocess (the reporter's suggested deterministic repro, cross-platform).Validation
pipshim on PATH)_run()vs pipe-holding grandchild sleeping 20stimeoutin ~1stests/tools/test_env_probe.pyscripts/run_tests.shtests/run_agent/test_run_agent.py(probe wiring)Fixes #67964
Infographic