Skip to content

Keep an OOM verdict for a corpse first seen past its deadline - #2065

Merged
ppXD merged 1 commit into
mainfrom
fix/keep-oom-verdict-for-a-corpse-past-its-deadline
Sep 30, 2026
Merged

ppXD merged 1 commit into
mainfrom
fix/keep-oom-verdict-for-a-corpse-past-its-deadline

Conversation

@ppXD

@ppXD ppXD commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • The observe loop read the wall clock before asking whether the supervisor was still there, so a run whose supervisor was already gone and whose deadline had already passed was stopped as TimedOut on the clock alone. That corpse never reached VanishedAsync, which classifies one by its exit marker, then a controller's deadline stop record, then the cgroup OOM counter, and only turns a plain Failed into TimedOut. An OOM-killed run first observed after its deadline (a re-attach after an outage, a slow worker) read TimedOut instead of ResourceExhausted and was retried under the same memory ceiling.
  • The clock now stops a run by itself only while the supervisor is not provably gone (!IsSupervisorGone(handle): still running, or a pid this worker cannot resolve, which IsSupervisorGone already answers false for). A supervisor that is gone falls through to the existing VanishedAsync call, which already applies the clock to a plain failure. The deadline stop record and the exit marker are still read first, unchanged.

Test plan

  • Unit: LocalProcessDurableRunnerTests 154 -> 157, all green. Three new tests drive the observe loop through AttachAsync instead of handing VanishedAsync the corpse: an OOM-killed supervisor past its deadline is ResourceExhausted; the same corpse without an OOM count is TimedOut; a live supervisor past its deadline is still TimedOut and killed (its launch deadline is 120 s away, beyond the test's 60 s bound, so only the observer's clock can settle it).
  • Before the fix: the OOM row failed with TimedOut and the other two passed.
  • Mutation: restoring the unconditional clock check turns the OOM row red (1 red, 2 green) on macOS and on Linux. Inverting the condition (&& IsSupervisorGone(handle)) turns the OOM row and the live-supervisor test red. Each mutation was restored by copy and verified with cmp and git diff.
  • Linux (aarch64 container, root): the whole LocalProcessDurableRunnerTests class, 157/157.
  • Neighbours: NativeLaunchRegistryTests 91/91 (including the NativeTerminateOutcomeTests.cs partial), LocalProcessLaunchDiscoveryCounterexampleTests 3/3, CgroupResourceLimitTests 6/6.

The observe loop read the wall clock before asking whether the
supervisor was still there, so a run whose supervisor was already gone
and whose deadline had already passed was stopped as TimedOut on the
clock alone. That corpse never reached VanishedAsync, which classifies
one by its exit marker, then a controller's deadline stop record, then
the cgroup OOM counter, and only turns a plain Failed into TimedOut.
An OOM-killed run first observed after its deadline (a re-attach after
an outage, a slow worker) therefore read TimedOut instead of
ResourceExhausted, and was retried under the same memory ceiling.

The clock now stops a run by itself only while the supervisor is not
provably gone: still running, or a pid this worker cannot resolve (a
handle another host minted), which IsSupervisorGone already answers
false for. A supervisor that is gone falls through to VanishedAsync,
which already applies the clock to a plain failure. The deadline stop
record and the exit marker are still read first, unchanged.

The tests drive the observe loop itself instead of handing VanishedAsync
the corpse: an OOM-killed supervisor past its deadline is
ResourceExhausted, the same corpse without an OOM count is TimedOut, and
a live supervisor past its deadline is still timed out and killed.
@ppXD
ppXD merged commit 87c5b3b into main Sep 30, 2026
6 checks passed
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