[front] fix: keep the reaper from sleeping a sandbox with an exec in flight - #30197
Open
fontanierh wants to merge 4 commits into
Open
[front] fix: keep the reaper from sleeping a sandbox with an exec in flight#30197fontanierh wants to merge 4 commits into
fontanierh wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
fontanierh
force-pushed
the
fn-reaper-exec-guard
branch
from
August 10, 2026 13:47
44c141d to
49456b3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The lifecycle lock serializes sandbox transitions but has never covered a running exec, so the reaper can pause a sandbox while an exec is still committing pod-state writes. The sleep flow flushes SQLite state to the GCS replica, pauses at the provider, then flips status — and an exec whose writes land after the flush loses them silently, because sleepers are later destroyed without a re-flush. Flagged during review of #30194, which widened admission into this window; the window itself predates it.
Fix: exec-activity counters in Redis, consulted by the flows that flush and then transition.
SandboxResource.execbrackets every workload exec with a monotonicstartedincrement and a matchingfinishedone, each record an atomic MULTI that refreshes both keys' TTLs so the pair cannot drift apart (execRoot is excluded: root commands are lifecycle plumbing, including the pre-sleep flush itself).dangerouslySleepIfRunningand the kill sweep'sdangerouslyDestroyIfKillRequested— where losing the race is permanent, since a destroyed sandbox is never re-flushed — skip their transition when an exec is in flight before the flush, and re-check after it —startedbeing monotonic catches even an exec that began and finished entirely during the flush, which an in-flight gauge alone would miss (and whichlastActivityAtcannot signal, being throttled to one write per 30s). Skipping is not an error: status stays running and the reaper retries next cycle, exactly like a pod that was never idle. An unreadable signal fails closed the same way.The counters are best-effort on the write side (an exec never fails because Redis hiccuped) and both expire 10 minutes after the last record, so a replica that dies mid-exec un-wedges the sleep via TTL rather than a count that never settles. Deferring a kill is safe: a busy pod's next invocation escalates through ensureActive's kill-requested branch, which recreates on access regardless. A residual window remains between the post-flush read and the provider pause — milliseconds against the previous seconds-long flush — and
pauseForApprovalkeeps today's behavior, since aborting an approval pause has UX cost and the approval flow's own exec is blocked waiting.Tests
sandbox_resource suite: quiet sandbox sleeps with the activity read on both sides of the flush, in-flight exec skips the sleep before flushing anything, an exec that started and finished during the flush skips the pause, unreadable signal fails closed, an in-flight exec defers the kill-sweep destroy, and exec brackets start/end records including when the provider throws. front typechecks clean.
Risk
False positives only delay a sleep by one reaper cycle. A lost
finishedrecord blocks sleep for at most the 10-minute TTL. The exec-side Redis cost is two INCR+EXPIRE round trips per exec, off the latency-critical path's measured contention point.Deploy Plan
Normal deploy. No migration, no flag. Worth watching the reaper's "not sleeping" log lines for a stuck sandbox that never sleeps, which would indicate a leaked in-flight count before its TTL.