fix(windows): add CREATE_BREAKAWAY_FROM_JOB to windows_hide_flags() - #55753
fix(windows): add CREATE_BREAKAWAY_FROM_JOB to windows_hide_flags()#55753ms-alan wants to merge 1 commit into
Conversation
Closes NousResearch#55604 windows_hide_flags() was returning only _CREATE_NO_WINDOW, missing _CREATE_BREAKAWAY_FROM_JOB which is already used by windows_detach_flags() for the same reason: Electron and Tauri desktop apps wrap their subprocesses in job objects, and without the breakaway flag a child process (e.g. cmd.exe windows flash) can be killed when the parent Electron/Tauri process exits. Note: DETACHED_PROCESS is intentionally NOT added — windows_hide_flags() is for short-lived synchronous children where stdout capture is needed.
Duplicate of #55613 — identical core change ( |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the existing breakaway constant and keeping DETACHED_PROCESS out of the synchronous path.
Problems
hermes_cli/_subprocess_compat.py:138-144documents thatCREATE_BREAKAWAY_FROM_JOBcan causeERROR_ACCESS_DENIED/PermissionErrorwhen the parent job rejects breakaway. This change adds that bit to everywindows_hide_flags()consumer, but generic callers such astools/environments/base.py:145-153pass the result directly tosubprocess.Popenwithout the documented retry-without-breakaway path.- The PR has no regression test for the changed helper behavior. The linked canonical duplicate, #55613, includes a helper-level test.
Suggested changes
- Scope breakaway to spawns that actually need to outlive the parent job, or provide and test a safe fallback for hide-only callers before changing the shared helper.
- Add a focused simulated-Windows regression test for the final behavior.
Automated hermes-sweeper review.
| # objects, and without breakaway a child started with capture_output=True | ||
| # can be killed when the parent Electron/Tauri process exits — even though | ||
| # DETACHED_PROCESS is not set and stdio is still connected. | ||
| return _CREATE_NO_WINDOW | _CREATE_BREAKAWAY_FROM_JOB |
There was a problem hiding this comment.
CREATE_BREAKAWAY_FROM_JOB can make CreateProcess fail when the parent job disallows breakaway; current main documents this at hermes_cli/_subprocess_compat.py:138-144. Because this helper is used by synchronous callers without an OSError retry (for example tools/environments/base.py:145-153), please avoid applying this bit globally unless the fallback is designed and covered.
|
Closing after the class-level fix in PR #70205 (merged as 0dbf639) — the premise here doesn't hold against Win32 semantics, and the underlying symptom is addressed at a different layer. Two problems with adding
If a flash persists at a specific spawn site on current main, that's worth a fresh issue with the process name from the flash — it would point at a leg the parent-console fix doesn't cover. Thanks for digging into the Windows spawn behavior. |
Closes #55604
Summary
windows_hide_flags()was returning only_CREATE_NO_WINDOW, missing_CREATE_BREAKAWAY_FROM_JOBwhich is already used bywindows_detach_flags()for the same reason: Electron and Tauri desktop apps wrap their subprocesses in job objects, and without the breakaway flag a child process (e.g. cmd.exe windows flash) can be killed when the parent Electron/Tauri process exits.Root cause
CREATE_BREAKAWAY_FROM_JOB(0x01000000) already exists at line 110 of_subprocess_compat.pyand is used bywindows_detach_flags(). It was simply missing fromwindows_hide_flags().Fix
Added
_CREATE_BREAKAWAY_FROM_JOBtowindows_hide_flags():Note:
DETACHED_PROCESSis intentionally not added —windows_hide_flags()is for short-lived synchronous children where stdout capture is needed, andDETACHED_PROCESSwould sever stdio.Related