feat(container-runner): report actors as crashed on unexpected platform SIGTERM

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/rivet

• #5548 feat(container-runner): log version and git sha at actor sta …
• #5549 feat(container-runner): setup instance memory and cpu usage …
• #5546 fix(rivetkit): keep actor event loop alive through shutdown …
• #5539 feat(container-runner): report actors as crashed on unexpect … 👈
• #5536 feat(container-runner): log unexpected platform SIGTERM as a …
• #5530 fix(rivetkit-core): cancel driver alarm before sqlite teardo …
• #5531 feat(container-runner): keep instance warm instead of self-e …
• #5533 chore(container-runner): default stop grace to 10s
• #5528 fix(rivetkit): log and surface actor startup failures
• #5529 fix(rivetkit): treat empty actor input bytes as absent
• main

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

_{change lzuymmyk}

[claude]

Review

Re-checked this PR: the diff is still byte-identical to what was reviewed previously (same commit 3f1016bca0). The findings below still apply and don't appear to have been addressed yet.

Correctness gap: actors mid-on_start at the moment of SIGTERM aren't reported as crashed

register_ctx runs at the very end of on_start, right after the child spawn and children() insert succeed (actor.rs:166). But Ctx::stop_with_error (called from crash_all_actors, actor.rs:264-281) requires lifecycle_started == true on the core side (rivetkit-core/src/actor/context.rs:551-557), which only flips once the framework transitions the task to Started — strictly later than on_start returning.

So there's a real (if narrow) window where a Ctx is already in ACTOR_CTXS but lifecycle_started is still false. A platform SIGTERM landing there makes stop_with_error return Err(ActorLifecycleError::Starting), which crash_all_actors only logs at debug! and otherwise swallows:

for ctx in ctxs {
    if let Err(err) = ctx.stop_with_error(message) {
        tracing::debug!(...); // silently dropped
    }
}

That actor falls through to the plain drain/sweep path instead of being reported as crashed — undercutting the goal of this PR for exactly the actors most likely to be caught by an OOM: freshly started ones. Given the repo's fail-by-default logging guidance, worth either:

• Distinguishing the Starting failure (a real, actionable gap) from Stopping (expected/idempotent, e.g. a race with a normal concurrent destroy) and logging the former at warn!, or
• If this window is judged negligible in practice, leaving a one-line comment on crash_all_actors noting it's a known/accepted gap so a future reader doesn't assume full coverage.

Minor: duplicated magic message

"...likely OOM or running longer than 60 minutes" is hardcoded in two places: the signal-handler tracing::error! (main.rs:449,455) and the crash_all_actors call (main.rs:378). A shared const would keep the two from drifting apart if one gets edited later.

Test coverage

No tests cover the new ACTOR_CTXS registration/crash-reporting logic. The host e2e suite (container-runner/examples/e2e-test/host/run-host-tests.sh) already has "Scenario 7: container-runner shutdown stops its children," which SIGTERMs the runner — the exact trigger path for crash_all_actors — but doesn't assert anything about the new crash-report behavior, and there's no success-path log line to assert on (only the debug! failure log noted above). This is consistent with the rest of this binary's signal/process-shutdown paths (stop_all_children, spawn_signal_handler), which are also untested since they're inherently process/signal-driven, so it's not a blocker — but since the new behavior has no observable success signal in logs, it's effectively unverifiable by that scenario even if someone wanted to extend it. Consider an info-level log per actor on a successful stop_with_error call, both for production observability and to make this testable.

Nits

• register_ctx's doc comment ("Overwrites any stale entry left by a prior generation") is accurate given the duplicate-start branch reuses the new generation's Ctx for the still-running child.
• The retain_async drain-then-iterate pattern in crash_all_actors mirrors stop_all_children in main.rs — good consistency with the existing style.
• crash_all_actors runs before runtime.shutdown(), so its stop_with_error calls (spawned as tracked shutdown tasks) ride the existing drain window instead of needing their own timeout — correctly reuses the established shutdown machinery rather than adding new synchronization.

Overall this is a safe net improvement over silent reallocation on OOM/platform reclaim. The on_start race is the one thing worth a second look at (or an explicit "known gap" note) before merging.