Skip to content

Commit fa14e6d

Browse files
committed
fix(hooks): Windows has no polite stop, and asking for one killed the console
`background_stop` opened with what looked like the symmetric counterpart of the POSIX side's SIGTERM-then-grace: ::GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, ::GetProcessId(procH)); That call addresses a process GROUP attached to the CALLER's console, not a process. When the id does not name a live group of ours — and it does not, once the child has already exited, which `start /b`-style commands do immediately — the event reaches everything sharing that console instead. Measured on the Windows e2e runner: the entire suite died eleven seconds into the hooks test with exit code -1073741510 (0xC000013A, STATUS_CONTROL_C_EXIT) and printed no summary at all, because mcpp had sent Ctrl-Break to its own console. Nothing local can show this: the branch is #if'd out everywhere the code is developed, and the failure is not in the feature under test — it is the harness dying. The design document already said Windows has no graceful stop for a child with no console and no window of its own. The implementation did not believe it. The call is removed, `graceMs` is explicitly unspent on that platform, and the asymmetry with POSIX is stated in the declaration instead of faked with a call that reaches too far. The job object was always the mechanism. macOS ARM64 and Linux both ran the new e2e green on the previous commit (317_project_build_hooks.sh, 21s on macOS), so the POSIX half — process groups, the grandchild kill, and Ctrl-C cleanup — is confirmed on two OSes.
1 parent dd6a5f6 commit fa14e6d

2 files changed

Lines changed: 45 additions & 16 deletions

File tree

.agents/docs/2026-08-30-project-build-hooks-owned-intervals.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,17 @@ Windows: the job object is closed, which terminates the tree at once. There is
159159
no graceful equivalent that does not require enumerating the job's processes
160160
and posting window messages; the asymmetry is stated rather than hidden.
161161
162+
⚠️ The obvious symmetry — "ask with `GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT,
163+
pid)` first" — was written, and it is wrong in a way no local run shows. That
164+
call addresses a process GROUP attached to the caller's console, not a process.
165+
When the id does not name a live group of ours (and it does not, once the child
166+
has exited — `start /b`-style commands exit immediately) the event reaches
167+
everything sharing the console. Measured on the Windows e2e runner: the entire
168+
suite died eleven seconds into the hooks test with exit code `-1073741510`
169+
(`0xC000013A`, `STATUS_CONTROL_C_EXIT`) and printed no summary, because mcpp had
170+
sent Ctrl-Break to its own console. The design said Windows has no graceful stop;
171+
the first implementation did not believe it. The code now matches the design.
172+
162173
## Output
163174
164175
A self-closing hook inherits stdio, which is safe because nothing else is

modules/platform/src/windows/bounded_process.cppm

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,15 @@ BackgroundChild spawn_background(const char* commandLine,
129129
// player finished the track" from "the command does not exist".
130130
int background_running(unsigned long long process, int* exitCode);
131131

132-
// Closes the job, which terminates the whole tree at once. `graceMs` is
133-
// accepted for signature parity with the POSIX side and is spent waiting for
134-
// the child to leave on its own after the console-control event; Windows has
135-
// no portable graceful stop for a process with no window and no console of its
136-
// own, and pretending otherwise would be a promise this cannot keep.
132+
// Closes the job, which terminates the whole tree at once.
133+
//
134+
// `graceMs` is accepted for signature parity with the POSIX peer and is NOT
135+
// spent: Windows has no portable graceful stop for a child with no console and
136+
// no window of its own, and the call that looks like one
137+
// (GenerateConsoleCtrlEvent) addresses a process group attached to THIS
138+
// console — see the implementation for what that cost. The asymmetry with the
139+
// POSIX side's SIGTERM-then-grace is real and is declared here rather than
140+
// papered over.
137141
void background_stop(unsigned long long job, unsigned long long process,
138142
long long graceMs);
139143

@@ -474,17 +478,31 @@ void background_stop(unsigned long long job, unsigned long long process,
474478
static_cast<std::uintptr_t>(job))
475479
: nullptr;
476480

477-
// Ask first. A child created with CREATE_NEW_PROCESS_GROUP is its own
478-
// group, so this reaches it and nothing else — including not reaching
479-
// mcpp, which is the reason the group exists.
480-
if (procH && graceMs > 0) {
481-
::GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, ::GetProcessId(procH));
482-
::WaitForSingleObject(procH, static_cast<DWORD>(graceMs));
483-
}
484-
485-
// Then take the tree. Closing a KILL_ON_JOB_CLOSE job is the whole
486-
// mechanism: TerminateProcess on the child alone would leave whatever it
487-
// started behind.
481+
// ⚠️ NO POLITE ASK HERE, AND `graceMs` IS DELIBERATELY UNSPENT.
482+
//
483+
// The obvious "ask first" is
484+
//
485+
// ::GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, ::GetProcessId(procH));
486+
//
487+
// and it is wrong in a way that does not show up locally. That call
488+
// addresses a process GROUP attached to this console, not a process; when
489+
// the id does not name a live group of ours — which it does not, once the
490+
// child has already exited, and `start /b`-style commands exit at once —
491+
// the event reaches everything sharing the console instead. Measured on
492+
// the Windows e2e runner: the whole suite died eleven seconds into the
493+
// hooks test with exit code -1073741510 (0xC000013A,
494+
// STATUS_CONTROL_C_EXIT) and printed no summary at all, because mcpp had
495+
// Ctrl-Break'd its own console.
496+
//
497+
// Windows has no portable graceful stop for a child with no console and no
498+
// window of its own. The job IS the mechanism; the POSIX peer's
499+
// SIGTERM-then-grace has no equivalent here, and the asymmetry is stated
500+
// in the declaration rather than faked with a call that reaches too far.
501+
(void)graceMs;
502+
503+
// Closing a KILL_ON_JOB_CLOSE job takes the whole tree. TerminateProcess
504+
// on the child alone would leave whatever it started behind — which for
505+
// `start /b cmd /c player` is the player.
488506
if (jobH) ::CloseHandle(jobH);
489507
if (procH) ::CloseHandle(procH);
490508
}

0 commit comments

Comments
 (0)