Skip to content

Avoid signaling recycled process groups - #193

Merged
austinywang merged 2 commits into
mainfrom
issue-9573-pgid-reuse-safety
Aug 9, 2026
Merged

austinywang merged 2 commits into
mainfrom
issue-9573-pgid-reuse-safety

Conversation

@austinywang

@austinywang austinywang commented Aug 9, 2026 •

Copy link
Copy Markdown

After the direct child has been reaped, stop trusting its cached numeric process-group id because the kernel may reuse it. Teardown now signals only a foreground group freshly observed through the retained PTY. Includes regression coverage for an unrelated cached group and for reaping a distinct foreground job after the leader has genuinely exited.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Prevents teardown from signaling a recycled process group after the child is reaped. Now we only stop the foreground group read from the retained PTY, avoiding signals to unrelated jobs.

  • Bug Fixes
    • Stop trusting cached process_group_id once the child is reaped; use fresh foreground_process_group_id from the PTY.
    • Switch to killProcessGroupWithTimeouts to target only the active foreground group.
    • Add regression tests for ignoring stale groups and for killing a distinct foreground job after the leader exits.

Written for commit bc7e9f7. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved subprocess cleanup after a process exits.
    • Prevented teardown from targeting stale process information, reducing the risk of terminating unrelated foreground processes.
    • Ensured remaining subprocesses are consistently cleaned up when their leader exits.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026 •

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3a1827d3-cefa-49ba-a7ea-763b464e6521

📥 Commits

Reviewing files that changed from the base of the PR and between f66cfbd and bc7e9f7.

📒 Files selected for processing (1)
  • src/termio/Exec.zig

📝 Walkthrough

Walkthrough

Teardown now avoids stale cached process-group IDs after child reaping. New tests verify that unrelated groups remain alive and that foreground descendants are cleaned up after synchronous leader reaping.

Changes

Process-group teardown safety

Layer / File(s) Summary
Foreground process-group teardown
src/termio/Exec.zig
Teardown uses the foreground process group from the retained PTY when the process is externally absent.
Reaping and descendant cleanup tests
src/termio/Exec.zig
Tests cover stale cached groups and synchronous reaping before teardown. The distinct foreground-group test removes its concurrent reaper thread.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: mitchellh, rockorager

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-9573-pgid-reuse-safety

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@austinywang
austinywang merged commit 90ba327 into main Aug 9, 2026
105 of 106 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant