Skip to content

fix(daemon): launcher.go call sites still use fragile Getpgid rediscovery path #861

Description

@PierrunoYT

Context

Follow-up from review of PR #774, which introduced TerminateProcessGroup/terminateOwnedProcess specifically to avoid the fragile Getpgid rediscovery in TerminateProcessTree (documented at internal/execution/process_unix.go:62-69: on Darwin, Getpgid can return ESRCH once an unreaped group leader has exited, even though live descendants remain in the group it configured, causing TerminateProcessTree to silently fall back to signalling only the dead leader PID and leaving descendants running).

Two call sites still hold an *exec.Cmd that went through background.ConfigureChildProcessGroup (so launch-time group identity is known) but call background.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:493 is 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 call background.TerminateCommand(cmd) (or a comparable helper that uses terminateOwnedProcess) instead of background.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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't workingissue-approvedReviewed and approved by the core team; community PRs may implement this issue.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions