feat(container-runner): exit process when the last child stops

[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 5587
Push local edits: forklift submit
Merge when ready: forklift merge 5587

_{change ssuvmoso}

[claude]

Review

Small, well-scoped change: stop_child now self-exits the process once children() is empty instead of always staying warm. The reasoning in the updated comments is clear.

Potential issue: race between a new placement and last-child self-exit

stop_child (container-runner/src/actor.rs:39-65) removes the stopping actor from children(), awaits child.stop(effective_stop_grace()) (up to SIGTERM_BUDGET, several seconds), and only then checks children().is_empty() to decide whether to request_exit.

Meanwhile on_start (container-runner/src/actor.rs:116-230) only inserts the new child into children() after ChildProcess::spawn succeeds, which includes waiting for the child to become ready (readiness_timeout, default 30s).

If the engine ever places a second actor on this same instance while the last existing actor is stopping, there's a window (last child's removal through its stop() completing) where children() can look empty even though a new actor's on_start is mid-spawn but hasn't inserted into the registry yet. stop_child would then call request_exit, and main proceeds straight to stop_all_children + runtime.shutdown(), tearing down the whole process (PID 1) out from under the in-flight placement, rather than just failing that one actor's start.

In the recommended single-actor-per-instance configuration (child.rs says: "container-runner hosts one actor per container, configure the serverless runner with max_concurrent_actors=1 and platform request concurrency=1") this can't happen. But the code is explicitly written to support multiple concurrent actors per instance (the else branch here, plus main.rs's "hosts as many concurrent actors as the engine places on it"), so it's worth confirming whether the engine can ever route a new placement onto an instance whose last actor is concurrently stopping. If it can, this is a reliability regression introduced by moving from "always stay warm" to "self-exit on empty": a legitimate new placement would get killed and have to be retried elsewhere.

Stale doc comments elsewhere in the same files

The module-level doc comments at the top of both changed files still describe the old "always stays warm, never self-exits" behavior and now contradict the logic a few lines below:

• container-runner/src/main.rs:15-17: "The instance stays warm after its last actor stops and never self-exits; the engine reaps it by draining the /start connection once the request lifespan elapses, or the platform sends a SIGTERM."
• container-runner/src/actor.rs:7: "on_destroy stops the child while the instance stays warm for the next placement."

Since this PR's whole point is changing that exact behavior, these top-of-file summaries should be updated in the same change so they don't mislead the next reader. (The comment at actor.rs:204-206, "the instance down. The container stays warm and ready for the next placement...", is still accurate since a failed start doesn't call stop_child, no change needed there.)

Minor: dropped log-agent-drain rationale

The removed comment in actor.rs explained why the instance previously stayed warm: to give the log agent time to drain stderr before a fast self-exit could lose logs ("This keeps the serverless container long lived enough for the log agent to drain its stderr, which a fast self-exit could otherwise lose"). The new code exits as soon as the last child stops, with no equivalent delay. If that concern is now considered obsolete (e.g. because the actor-driven exit path drains runtime.shutdown() unbounded and that's enough time in practice), it'd be worth a one-line comment saying so; otherwise this looks like a reintroduction of the original problem.

Nit

Two actors stopping near-simultaneously and both calling request_exit is harmless since CancellationToken::cancel() is idempotent, so no correctness issue there, just noting it was considered.

Test coverage

No tests were added for this lifecycle change. The crate doesn't have existing integration coverage for actor start/stop sequencing (only tests/inline/boot_id.rs), so this may be consistent with current practice, but a test exercising "last child stop leads to process exit" (and ideally the concurrent-placement race above, if reachable) would help guard this behavior going forward.