Skip to content

fix(child_process): release timeout threads after child exit - #9640

Closed
proggeramlug wants to merge 3 commits into
mainfrom
fix/9592-spawn-timeout-thread
Closed

fix(child_process): release timeout threads after child exit#9640
proggeramlug wants to merge 3 commits into
mainfrom
fix/9592-spawn-timeout-thread

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #9592

Summary

  • replace the unconditional child timeout sleep with a cancellable channel deadline
  • have both spawn and async exec waiters cancel their timeout worker before publishing the exit event
  • add unit and end-to-end coverage for 50 immediate children plus a genuine timeout kill

Validation

  • focused release runtime timeout tests: 2 passed
  • full perry-runtime unit suite: 3,035 passed, 4 ignored
  • pinned-Node parity fixture: PASS (1/1)
  • pre-tag quick checks: PASS
  • pre-fix artifact sabotage check: thread-release assertion fails while the real timeout arm still passes

The broader affected-crate runner reaches an unrelated existing perry binary test failure: PERRY_CONCAT_SITE_CACHE is absent from the build-cache env inventory (introduced by #9514); this branch does not touch that code.

Summary by CodeRabbit

  • Bug Fixes

    • Timed child-process operations now release waiting resources promptly when the child exits.
    • Timeout handling continues to terminate children when configured deadlines are reached.
  • Tests

    • Added coverage for rapid child completion, timeout enforcement, and expected process termination behavior.
  • Documentation

    • Added a changelog entry describing the improved timed child-process behavior.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 91e0aa0d-1361-431d-9942-37c96a2562c9

📥 Commits

Reviewing files that changed from the base of the PR and between d81e441 and 734630d.

📒 Files selected for processing (1)
  • test-files/test_gap_9592_child_timeout_threads.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test-files/test_gap_9592_child_timeout_threads.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Timed child-process waits now cancel their timeout workers when the child exits. Spawn and exec paths pass timeout data into the waiter. Tests verify prompt thread release and deadline-based termination.

Changes

Child timeout cancellation

Layer / File(s) Summary
Waiter-owned timeout cancellation
crates/perry-runtime/src/child_process/reactor/timeout.rs
cp_spawn_waiter coordinates child completion and timeout waiting through a cancellation channel. Runtime tests cover early cancellation and elapsed deadlines.
Spawn and exec timeout integration
crates/perry-runtime/src/child_process/reactor.rs, test-files/test_gap_9592_child_timeout_threads.ts, changelog.d/9640-child-timeout-threads.md
Spawn and exec paths pass timeout data to cp_spawn_waiter. The regression test checks thread counts after 50 quick children and verifies that a slow child receives SIGTERM within the timeout window. The changelog records the behavior.

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

Merge Risk: ⚪ Minimal · up to 73463

Child-process timeout workers are intended to stop promptly after child exit while preserving timeout termination behavior. No current merge-blocking risk is identified.

Sequence Diagram(s)

sequenceDiagram
  participant ChildProcessAPI
  participant cp_spawn_waiter
  participant ProcessWaiter
  participant TimeoutWaiter
  ChildProcessAPI->>cp_spawn_waiter: pass timeout and kill signal
  cp_spawn_waiter->>ProcessWaiter: wait for child exit
  cp_spawn_waiter->>TimeoutWaiter: wait until deadline
  ProcessWaiter->>TimeoutWaiter: send cancellation on child exit
  ProcessWaiter->>ChildProcessAPI: emit CpEvent::Exited
  TimeoutWaiter->>ChildProcessAPI: emit CpEvent::Timeout on deadline
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: releasing child-process timeout threads after child exit.
Description check ✅ Passed The description provides the issue reference, change summary, concrete implementation details, and validation results. It does not use every template heading, but it is mostly complete and covers the …
Linked Issues check ✅ Passed The changes address issue #9592 by making timeout waits cancellable on child exit while preserving deadline-based termination. The added unit and end-to-end tests cover 50 immediate children and a gen…
Out of Scope Changes check ✅ Passed The changelog entry, runtime changes, and test coverage all support the linked issue objectives. No unrelated code changes are identified.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.
Full details: Description check

Explanation

The description provides the issue reference, change summary, concrete implementation details, and validation results. It does not use every template heading, but it is mostly complete and covers the required information.

Full details: Linked Issues check

Explanation

The changes address issue #9592 by making timeout waits cancellable on child exit while preserving deadline-based termination. The added unit and end-to-end tests cover 50 immediate children and a genuine timeout kill.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/9592-spawn-timeout-thread

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.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed via merge train #9650 (rebase-merge, authorship preserved).

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.

cp_spawn_timeout leaks one OS thread per timed spawn: sleeps the full timeout, never observes child exit — 12 live in the field capture

1 participant