Repository navigation
Conversation
A stop with forceKillAfter sent SIGTERM to the process group and escalated to SIGKILL only while the leader was still running. When the leader exited on SIGTERM, any child that outlived the signal kept running, and could keep starting more processes, with nothing tracking the group. After the first signal, wait for the whole group to empty. When the grace period ends, send SIGKILL to the group until it has no members, for at most one second.
There was a problem hiding this comment.
Requesting changes: the fix works on a normal Linux host, but it adds a delay of about 4 s to every shell timeout or abort when opencode runs as PID 1, which is what the official Docker image does.
How I checked
- In a PID namespace whose init reaps processes, the new test fails on the base and passes with this change. The whole spawner test file also passes there.
- In this sandbox, PID 1 doesn't reap orphaned processes, so dead group members stay as zombies.
kill(-pgid, 0)keeps succeeding on zombies, so the group never looks empty. - In that setup, I timed
handle.kill({ forceKillAfter: "3 seconds" })onsh -c 'sleep 30 & echo $!; wait', wheresleepexits on SIGTERM. It takes 2 ms ondevand 4002 ms with this change: the full grace period, then the full 1 s SIGKILL loop. Under a reaping init it takes 2 ms. - The new test also fails in that setup.
Who is affected: packages/cli/Dockerfile uses ENTRYPOINT ["opencode"] with no init, so opencode is PID 1 and never reaps orphans (#53564). In that image, any timed-out or aborted command with a background child now costs forceKillAfter + 1 s, as does any scope release that goes through stop. Details and options are in the inline comment.
Public API: unchanged.
| // Descendants can outlive the leader in its group. A group ID stays reserved until its last member is reaped, so | ||
| // signalling the group until ESRCH stays on this group. | ||
| const grouped = globalThis.process.platform !== "win32" && (command.options.detached ?? true) | ||
| const untilGroupGone = (signal: NodeJS.Signals | 0) => |
There was a problem hiding this comment.
kill(-pgid, 0) also succeeds while a group member is a zombie. When no process reaps orphans, the group never reaches ESRCH. That's the case when opencode is PID 1, as in the official Docker image (#53564), or in containers without --init.
In that case, every stop with forceKillAfter waits the full grace period, then runs the SIGKILL loop until its 1 s cap. This happens even when every member already died on SIGTERM. I measured 2 ms on dev and 4002 ms here for sh -c 'sleep 30 & wait' with a 3 s forceKillAfter.
Some possible ways out:
- Don't count zombies as live members (on Linux, read the state in
/proc/<pid>/stat). - Once the leader has exited, send one SIGKILL to the group without waiting for ESRCH.
- Wait for the empty group only on the SIGKILL path, not during the grace period.
|
|
||
| yield* handle.kill({ forceKillAfter: 100 }) | ||
|
|
||
| expect(alive(group)).toBe(false) |
There was a problem hiding this comment.
This test assumes orphaned members get reaped. Where PID 1 doesn't reap them (for example a container with no init), they stay zombies, the group stays alive, and this assertion fails after about 1.1 s. It failed that way for me in a sandbox like that; it passes under a reaping init.
Issue for this PR
Related to #59006, which asks for a way to stop an agent's whole process tree. No issue covers this exact case, so this PR fixes one focused part of it: what a shell timeout or abort leaves behind.
Type of change
What does this PR do?
When a shell command times out or is aborted,
stopsends SIGTERM to the command's process group. It escalates to SIGKILL only if the leader (sh) is still running whenforceKillAfterends. The leader usually exits on SIGTERM at once, so the escalation never happens. A child that outlives the SIGTERM (one that handles or ignores it, or one started while the signal went out) keeps running with no parent, and nothing tracks the group any more. A command that keeps starting child processes can then run until the user's process limit is used up.With
forceKillAfterset,stopnow waits for the whole group to empty, not just the leader. If the grace period ends with members left, it sends SIGKILL to the group every 10 ms untilkillreports ESRCH, for at most one second. A group ID stays reserved while any member is alive, and the loop stops at the first ESRCH, so it only signals the command's own group.Every
stopwithforceKillAftergets this: the shell tool's timeout and abort, and scope release withRELEASE_FORCE_KILL_AFTER, for example a command that exits non-zero or an MCP stdio server. Windows (taskkill /T),detached: falseand stops withoutforceKillAfterare unchanged. A command that exits 0 still leaves its background children running, as before.This is separate from #61876, which moves shells into the process daemon. It works on the current spawner and doesn't depend on that PR.
How did you verify your code works?
packages/core/test/effect/cross-spawn-spawner.test.ts. The leader exits on SIGTERM while its child ignores SIGTERM and starts one shortsleepafter another (one at a time, for at most 10 s). It fails ondevbecause the group is still alive afterkill, and passes with this change. Checked on Linux x86_64 and macOS 27 arm64.packages/core, runbun run script/test.ts test/effect/cross-spawn-spawner.test.ts(33 pass, 1 skip, 0 fail) and the seven shell/MCP/process files listed in the evidence (206 pass, 24 skip, 0 fail). Runbun check --no-scriptandtsc --noEmitinpackages/util, andbun check --no-script -b -p tsconfig.tests.jsoninpackages/core; run Prettier and oxlint on the two changed files. The Linux and macOS before/after runs and the timeout transcript also pass.handle.kill({ forceKillAfter: "3 seconds" })as the shell tool's timeout does, and lists the group. Ondev,killreturns in 3 ms and two members are still there 4 s later, still starting newsleepprocesses. With this change,killreturns after the 3 s grace period and the group is empty. macOS gives the same result.Not checked: Windows, whose code path is unchanged.
Screenshots / recordings
No UI change. Below are test and runtime output from Linux, before and after.
The new test on
dev: it fails because the process group is still alive afterkill.The new test with this change: it passes.
A timeout on
dev:killreturns at once, and the group keeps two members that are still starting new processes 4 s later.A timeout with this change:
killreturns after the 3 s grace period, and the group is empty.Checklist
Generated with Claude Opus 5.5 and GPT-6.1 Sol in T3 Code Nightly
Added by the opencode bot (AI-generated).
Related: #59006, #53564