feat(container-runner): drain children and engine concurrently on SIGTERM

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/actors

• #5594 feat(container-runner): drain engine sleeps unless the idle …
• #5595 feat(container-runner): jitter idle timeout to avoid teardow …
• #5585 feat(container-runner): self-sleep on repeated actor start
• #5586 feat(container-runner): sleep on startup idle timeout
• #5589 chore(container-runner): tighten comments
• #5587 feat(container-runner): exit process when the last child sto …
• #5588 feat(container-runner): drain child before SIGTERM on engine …
• #5584 feat(container-runner): drain children and engine concurrent … 👈
• #5565 fix(envoy-client): ack terminating stop commands so pegboard …
• main

Get stack: forklift get 5584
Push local edits: forklift submit
Merge when ready: forklift merge 5584

_{change tpptkxqq}

[railway-app]

🚅 Deployed to the actors-pr-5584 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
frontend-cloud	😴 Sleeping (View Logs)	Web	Aug 26, 2026 at 6:37 pm
website	❌ Build Failed (View Logs)	Web	Aug 25, 2026 at 7:36 pm
kitchen-sink	😴 Sleeping (View Logs)	Web	Aug 22, 2026 at 2:35 pm
frontend-inspector	😴 Sleeping (View Logs)	Web	Aug 22, 2026 at 1:14 pm
ladle	✅ Success (View Logs)	Web	Aug 21, 2026 at 9:41 pm
mcp-hub	✅ Success (View Logs)	Web	Aug 21, 2026 at 9:40 pm

[claude]

Review: feat(container-runner): drain children and engine concurrently on SIGTERM

Overall this is a solid, well-documented improvement to the SIGTERM teardown path. The core idea, running the engine drain and the direct child sweep concurrently, each bounded by the full budget, instead of nesting them with hand-carved percentages, is a real fix for a latent bug in the old code: stop_all_children used to stop children serially in a for loop, so an instance hosting more than one child could blow past the 1s sweep grace (N children x grace each). The new join_all-based concurrent sweep fixes that. Nice cleanup of signal_drain_timeout/signal_child_stop_grace/SIGNAL_SWEEP_GRACE too, confirmed no dangling references remain and futures-util is already a workspace dependency, so the new join_all import should compile cleanly.

Observations (non-blocking)

1. Duplicate concurrent stop() on the same child is now the common case, not just the timeout edge case. Since stop_all_children snapshots CHILDREN via retain_async essentially immediately when tokio::join! starts, it will grab (and start stopping) nearly every child before the engine drain's per-actor on_destroy hook gets a chance to run. If that hook does complete in time, it calls child.stop() again on the same Arc<ChildProcess> (it reads from the actor's own self.child field, which the sweep doesn't touch). This is safe/idempotent (has_exited() check, watch-channel wait_exit(), and let _ = signal::kill(...) swallowing errors all tolerate the race, per the existing comment in stop_child), but it does mean duplicate SIGTERM/SIGKILL sends and duplicate log lines for the normal (non-timeout) case now, where previously this only happened when the drain timed out. Worth being aware of, not a functional bug.

2. Reduced timing margin. The default budget dropped from 10s to 9s, and both concurrent branches (drain, stop_all_children) are now each bounded by the full budget rather than a fraction of it. That trades away the old structural slack (drain capped at 60%, sweep at a fixed 1s, worst case ~7s of a 10s budget) for a single explicit 1s reservation (9s of an assumed ~10s budget), with serve_shutdown.cancel(); serve.await still running unbounded after the tokio::join! completes. Net effect: less slack before an assumed platform SIGKILL than before. Likely fine in practice since serve.await teardown should be fast, but worth confirming, and worth calling out in the PR/changelog that the default changed, for deployments relying on the implicit default rather than setting RIVET_SIGTERM_BUDGET_SECS explicitly.

3. No test coverage added for the new concurrent drain/sweep behavior or the budget math (only boot_id/input have inline tests today). Real SIGTERM/subprocess timing is admittedly awkward to unit test, but given this is a meaningful behavioral change to the shutdown path, it'd be worth confirming whatever e2e/manual test harness exists for this runner exercises a SIGTERM shutdown with multiple children, if it doesn't already.

Style / CLAUDE.md compliance

• No em dashes, tabs preserved, comments are complete sentences, consistent with repo conventions.
• No Mutex<HashMap>/Mutex<HashSet> introduced; existing scc::HashMap usage for CHILDREN/RESERVED_PORTS is untouched and correctly used.

No blocking issues found.