Reclaim Modal sandboxes whose create call was interrupted - #2406
Reclaim Modal sandboxes whose create call was interrupted#2406nicolaslara wants to merge 1 commit into
Conversation
Modal commits a sandbox and hands it to the scheduler before `Sandbox.create` returns, so a create interrupted in flight still boots and bills a sandbox — measured at roughly a second after the caller stopped waiting for it. `_sandbox` is only assigned once the call returns, so an interrupted `start` leaves nothing for `teardown` or the atexit backstop to act on, and the sandbox runs to its 24h maximum lifetime. Cancellation is the common trigger: Ctrl-C on an eval, or a rollout cancelled while its batch is still provisioning. `CancelledError` is not an `Exception`, so it also slipped past the existing handler and left no log line. The name is ours and unique per rollout, so an interrupted create is still addressable. Look it up on the way out and adopt it, and the owner's `stop` disposes of it like any other sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| import modal | ||
|
|
||
| with contextlib.suppress(Exception): | ||
| self._sandbox = await asyncio.shield( |
There was a problem hiding this comment.
[P2] Keep the recovered handle across repeated cancellation
A bare asyncio.shield re-raises when this task is cancelled again. That second CancelledError is not caught by suppress(Exception), so the assignment is skipped even if the shielded lookup later completes; _sandbox remains None and teardown still cannot reclaim the sandbox. verifiers.v1.utils.aio.run_shielded exists specifically to absorb repeated task cancellations until the owned operation completes. The self._sandbox assignment needs to happen inside the coroutine passed to that helper so the handle is installed before cancellation is re-raised.
| except ( | ||
| Exception | ||
| ) as e: # provisioning failure is one rollout's problem, not the eval's | ||
| await self._adopt_orphan() |
There was a problem hiding this comment.
[P1] Don't adopt a pre-existing sandbox after a duplicate-name error
Modal documents that Sandbox.create raises AlreadyExistsError when the requested name already exists. This broad except Exception then calls from_name, attaches to that already-running sandbox, and the owner's finally/abort path calls stop(), whose Modal teardown terminates the adopted handle. Because make_runtime and provision_runtime accept caller-supplied names, a duplicate-name create can therefore terminate a sandbox this runtime did not create. Please keep ownership of the in-flight create directly—for example, perform the create and self._sandbox assignment inside a coroutine passed to the existing run_shielded helper—instead of recovering by name after every exception.
Symptom
Interrupt an eval that uses the Modal runtime while sandboxes are still being provisioned, and more sandboxes stay alive than the harness ever reports as up. They then run until their 24h
timeoutexpires.Cause
Modal commits the sandbox and hands it to the scheduler before
Sandbox.createreturns. A create interrupted in flight therefore still produces a running, billed sandbox. Timeline measured on one such sandbox:In
ModalRuntime.startthe handle is only assigned once the call returns:So
_sandboxstaysNone, and both cleanup paths key off exactly that attribute —teardownreturns early, and thecleanupatexit backstop no-ops. The sandbox is unreachable from the process that made it.Cancellation is the common trigger (Ctrl-C, or a rollout cancelled while its batch is still provisioning), and
CancelledErroris not anException, so it also slipped pastexcept Exceptioninstartand left no log line. That is why the "sandbox ... up" count doesn't match what's actually running. A connection dropped on the reply has the same shape.Fix
The
namepassed toSandbox.createis ours and unique per rollout, and Modal enforces name uniqueness per app, so an interrupted create is still addressable. Look it up on the way out and adopt it; the owner'sstopthen disposes of it like any other sandbox._adopt_orphanis a no-op when a handle is already held, so it is also safe on the existingexcept Exceptionpath — which additionally covers a create that failed after the server created the sandbox.Verification
Real Modal sandboxes, 12 rollouts started concurrently, batch cancelled mid-create, then every runtime stopped exactly as the owner's
finallydoes.LEAKEDis measured withSandbox.listagainst the app, not inferred:Before
After
In run 1 after the fix, one rollout's create never reached the server (
created_server_side=11), so there was correctly nothing to adopt.Cancelling outside the create window is unaffected: the handle is already held and
_adopt_orphanreturns immediately.uv run ruff check,uv run ruff format --check, anduv run ty check verifiersall pass. No tests added, perAGENTS.md— the check above was a temporary script.Two notes for maintainers, not changed here
ModalConfig.creates_per_secdefaults to 40.0. Modal's default per-workspace sandbox creation limit is 5/s with a 150-token burst bucket, so this default is 8× over for a workspace that hasn't had its limit raised. Creates then retry with backoff, which keeps them in flight much longer and widens the window this PR closes. Worth either lowering the default or documenting that the limit needs raising.Sandbox.createacceptsidle_timeout. It is the only cleanup that survives losing the client entirely — a hard kill, or a second Ctrl-C that kills the event loop before any recovery code runs. Currently onlytimeout=24his set, so anything the process can't clean up lives for a day.Disclosure: I work at Modal. This came out of debugging a leak with a user running verifiers on us.
🤖 Generated with Claude Code