fix(watch): stop nub watch leaking an immortal node --watch supervisor - #620
Merged
Conversation
run_watch was the only long-lived spawn in nub that called Command::status() directly. Every other one — spawn.rs:672, spawn.rs:1182, cli.rs:4647 — goes through a process-group + reaper path. That single gap is why orphan sweeps on the dev host found 99 stranded 'node --watch' processes, all PPID=1, the oldest 17 hours, and zero stranded rustc or cargo. It matters most precisely here: node --watch is a supervisor that by design never exits, and it spawns a watched grandchild of its own. With no group and no reaper, the death of the nub leader — a killed agent session, a test harness that gives up, a closed terminal — reparents both to launchd, where they run forever holding an fsevents watch on a source tree. They accumulate monotonically, which is why they had to be cleared by hand. Both spawn sites now use status_forwarding_signals(), the existing helper documented as the signal-faithful, subtree-reaching equivalent of status(): own process group, the #480 SIGKILL-on-leader reaper held across the wait, and terminating signals forwarded to the whole subtree. The regression test kills the nub LEADER, not the group, and that distinction is the test. A first version killed the group and passed against unfixed code — on the unfixed path the watched node inherits nub's group, so a group signal reaches it directly and no reaper is exercised. Verified both directions: leader-kill fails without the fix, passes with it.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
colinhacks
added a commit
that referenced
this pull request
Jul 29, 2026
Two conflicts, both in vendor/aube/crates/aube-linker/, both the same collision: this branch retyped `disk_materialize` from HashSet<String> to PackageNameMatcher so `install.linker.eject` can take glob patterns, while #616 added a sibling `reusable_hoisted: HashSet<String>` to let the hoisted pass skip unchanged packages. Kept both — the matcher type and main's new field with its rationale comment. Verified rather than inspected, since a clean auto-merge into non-compiling code has bitten this branch twice: cargo check across the workspace with --all-targets --all-features passes, and main's watch- supervisor fix (#620) survives at both status_forwarding_signals call sites in cli.rs with no bare status() left in run_watch.
Contributor
Author
|
Shipped in v0.7.0: https://github.com/nubjs/nub/releases/tag/v0.7.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
run_watchwas the only long-lived spawn in nub that calledCommand::status()directly. Every other one —spawn.rs:672,spawn.rs:1182,cli.rs:4647— goes through a process-group + reaper path.That single gap is why orphan sweeps on the dev host kept finding stranded processes:
Exclusively
node --watch. Never rustc, never cargo — because those paths are already guarded.Why this path matters most
node --watchis a supervisor that by design never exits, and it spawns a watched grandchild of its own. With no process group and no reaper, the death of the nub leader — a killed agent session, a test harness that gives up, a closed terminal — reparents both tolaunchd, where they run forever holding an fsevents watch on a source tree. They accumulate monotonically, so the only remedy was clearing them by hand once the machine slowed to a crawl.The fix
Both spawn sites (
cli.rs:4832NODE_COMPAT,cli.rs:~5084augmented) now usestatus_forwarding_signals()— the existing helper documented as "the signal-faithful, subtree-reaching equivalent ofcmd.status()": own process group, the #480 SIGKILL-on-leader reaper held across the wait, and terminating signals forwarded to the whole subtree.No new machinery. This is #480's reaper applied to the one path that was missing it.
The regression test kills the LEADER, not the group — and that is the test
A first version killed the process group and passed against unfixed code. On the unfixed path the watched node inherits nub's group, so a group signal reaches it directly and no reaper is exercised. It would have shipped green forever while protecting nothing.
Killing the leader alone is also the shape of the real failure: a session dies, nub goes with it,
node --watchdoes not.Verified both directions:
watched node (pid 19633) survivedGates
cargo check -p nub-cli·cargo fmt --check·cargo clippy --all-targets --all-features -- -D warnings·cargo test -p nub-cli --test pdeath_watch— all clean locally.Scope
This stops new orphans. It does not address the separate contention on that host (~20 concurrent builds on 10 cores, 74 live worktrees / 262 GB of target dirs) — those are structural and tracked separately.
Refs #480