Skip to content

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

Open
abcxff wants to merge 1 commit into
stack/feat-container-runner-log-unexpected-platform-sigterm-as-an-error-zsrurkrzfrom
stack/feat-container-runner-report-actors-as-crashed-on-unexpected-platform-sigterm-lzuymmyk
Open

feat(container-runner): report actors as crashed on unexpected platform SIGTERM#5539
abcxff wants to merge 1 commit into
stack/feat-container-runner-log-unexpected-platform-sigterm-as-an-error-zsrurkrzfrom
stack/feat-container-runner-report-actors-as-crashed-on-unexpected-platform-sigterm-lzuymmyk

Conversation

@abcxff

@abcxff abcxff commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review

Straightforward, well-scoped change: report actors as crashed (via stop_with_error) when the platform reclaims the container with an unexpected SIGTERM, instead of letting the reclaim look like a silent reallocation to the engine.

Correctness

  • The PLATFORM_RECLAIM / SIGNAL_SHUTDOWN ordering is correct: both are set before request_exit cancels EXIT, so by the time async_main wakes from EXIT.cancelled().await, effective_stop_grace() already observes the capped signal grace when crash_all_actors triggers on_destroystop_child.
  • Registering ACTOR_CTXS only after a successful ChildProcess::spawn (not immediately in on_start) correctly avoids leaking an entry for a generation whose start failed, since a failed start never runs on_destroy/on_sleep.
  • Using ctx.stop_with_error(...) (rather than ctx.destroy()) is the right primitive here — it reports StopCode::Error to the engine while still running the same local destroy-grace teardown.

Minor / nits

  • register_ctx does remove_async then insert_async as two separate awaits rather than an atomic upsert. The project CLAUDE.md calls out entry_async for atomic read-then-write on scc maps. In practice this is safe today because Pegboard's single-writer invariant means on_start for a given actor_id can't run concurrently with itself, so it's not an exploitable race today — just flagging as a small inconsistency with the project's own scc guidance in case that invariant ever loosens.
  • The "likely OOM or running longer than 60 minutes" explanation is now duplicated across three log/message sites (two pre-existing in spawn_signal_handler, one new in the crash_all_actors call) with slightly different wording each time. Worth pulling into a shared const so the phrasing (and the "60 minutes" figure, if the platform's actual budget ever changes) can't drift between the copies.
  • No tests were added for register_ctx/crash_all_actors/ACTOR_CTXS. The crate already has a tests/inline pattern (boot_id.rs); a small unit test asserting that a failed start leaves no ACTOR_CTXS entry, or that crash_all_actors drains the map, would pin down the invariant the comments describe.

Not a blocker, just noting for awareness: if the platform sends SIGTERM while an actor is mid-on_start (child process still spawning, before register_ctx runs), that actor won't get a crash report — it'll just be silently swept later by stop_all_children. That's a narrow, pre-existing gap (not introduced by this PR) and probably fine given engine-side start timeouts, but worth confirming it's an accepted limitation.

Overall this looks solid and matches the codebase's scc/async-lock/error-handling conventions. Nothing here blocks merging.

@abcxff
abcxff force-pushed the stack/feat-container-runner-report-actors-as-crashed-on-unexpected-platform-sigterm-lzuymmyk branch from 36380bb to d383183 Compare August 3, 2026 20:17
@abcxff
abcxff force-pushed the stack/feat-container-runner-log-unexpected-platform-sigterm-as-an-error-zsrurkrz branch from da132e5 to 94c1f83 Compare August 3, 2026 20:17
@abcxff
abcxff force-pushed the stack/feat-container-runner-log-unexpected-platform-sigterm-as-an-error-zsrurkrz branch from 94c1f83 to ae5019a Compare August 4, 2026 16:15
@abcxff
abcxff force-pushed the stack/feat-container-runner-report-actors-as-crashed-on-unexpected-platform-sigterm-lzuymmyk branch from d383183 to 2e35cdc Compare August 4, 2026 16:15
@abcxff
abcxff force-pushed the stack/feat-container-runner-log-unexpected-platform-sigterm-as-an-error-zsrurkrz branch from ae5019a to 4346f55 Compare August 5, 2026 17:09
@abcxff
abcxff force-pushed the stack/feat-container-runner-report-actors-as-crashed-on-unexpected-platform-sigterm-lzuymmyk branch from 2e35cdc to 3f1016b Compare August 5, 2026 17:09
@abcxff
abcxff force-pushed the stack/feat-container-runner-report-actors-as-crashed-on-unexpected-platform-sigterm-lzuymmyk branch from 3f1016b to 41fad8a Compare August 10, 2026 17:16
@abcxff
abcxff force-pushed the stack/feat-container-runner-log-unexpected-platform-sigterm-as-an-error-zsrurkrz branch from 4346f55 to 6733b2a Compare August 10, 2026 17:16
@abcxff
abcxff force-pushed the stack/feat-container-runner-report-actors-as-crashed-on-unexpected-platform-sigterm-lzuymmyk branch 2 times, most recently from ddda4b1 to a3dec62 Compare August 10, 2026 20:35
@abcxff
abcxff force-pushed the stack/feat-container-runner-log-unexpected-platform-sigterm-as-an-error-zsrurkrz branch from 69f80f4 to 9caf3ad Compare August 10, 2026 20:35
@abcxff
abcxff force-pushed the stack/feat-container-runner-log-unexpected-platform-sigterm-as-an-error-zsrurkrz branch from 9caf3ad to 823007e Compare August 10, 2026 22:09
@abcxff
abcxff force-pushed the stack/feat-container-runner-report-actors-as-crashed-on-unexpected-platform-sigterm-lzuymmyk branch from a3dec62 to 1b7821e Compare August 10, 2026 22:09
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.

1 participant