Repository navigation
fix(daemon): launcher.go call sites still use fragile Getpgid rediscovery path #861
Description
Activity
- added a commit that references this issue
on Sep 1, 2026 Labelled and approved. I checked the open PRs that touch the daemon (#927, #987) and neither reaches these two call sites, so nothing is in flight for this. The fix is what the issue already prescribes: the two sites hold an
*exec.Cmdthat went throughConfigureChildProcessGroup, so they should call the group primitive rather thanTerminateProcess(pid)and its rediscovery path. A regression that makes the leader exit unreaped before termination, then asserts the descendants are gone, is the test that proves it.Back in the pool, and still valid on current main.
This came up in an audit of open issues whose only linked pull request had closed, which made them look covered to a query while nobody was actually working them. The earlier attempt closed without landing, so this is open for a fresh one.
I verified the problem still exists rather than assuming it, and the audit verdict was independently re-checked before I posted this. The label is already on, so a PR implementing it can merge on review.
- added a commit that references this issue
on Sep 26, 2026
Context
Follow-up from review of PR #774, which introduced
TerminateProcessGroup/terminateOwnedProcessspecifically to avoid the fragileGetpgidrediscovery inTerminateProcessTree(documented atinternal/execution/process_unix.go:62-69: on Darwin,Getpgidcan returnESRCHonce an unreaped group leader has exited, even though live descendants remain in the group it configured, causingTerminateProcessTreeto silently fall back to signalling only the dead leader PID and leaving descendants running).Two call sites still hold an
*exec.Cmdthat went throughbackground.ConfigureChildProcessGroup(so launch-time group identity is known) but callbackground.TerminateProcess(pid), which routes through the fragile rediscovery path instead of the stronger primitive:internal/daemon/launcher.go:77(execWorker.Kill)internal/daemon/launcher.go:163(cmd.Cancel)internal/specialist/exec.go:493is correctly left alone since it only has a bare PID (no launch-time group knowledge to leverage).Proposed fix
Since both call sites hold the
*exec.Cmd, they can callbackground.TerminateCommand(cmd)(or a comparable helper that usesterminateOwnedProcess) instead ofbackground.TerminateProcess(cmd.Process.Pid), so they take the launch-time-invariant path rather than rediscovery.Not blocking; pre-existing behavior, same shape as the daemon-start path #774 fixed.