fix: free runtime resources on exit/interrupt - #1590
Merged
Conversation
A rollout's /tmp workspace is removed in `stop()`, but a process killed mid-rollout
(SIGKILL, OOM, hard crash, interrupted teardown) never reaches it, so the workspace
leaks with no way to reclaim it — repeated runs eventually fill /tmp ("No space left
on device" at mkdtemp).
Name each workspace `/tmp/v1-<pid>-*` and, once per process on the first `start()`,
sweep `/tmp/v1-<pid>-*` whose pid is no longer alive. PID-keyed, so a concurrent live
process's workspaces are never touched; graceful per-rollout cleanup (`stop()`) is
unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make resource cleanup a backend-agnostic property of `Runtime`: - a sync `cleanup()` is the teardown source of truth; the public async `stop()` runs it off the event loop on the happy path. - `make_runtime` registers each runtime in a WeakSet and arms one sync `atexit` hook that calls `cleanup()` on anything still live — so a Ctrl-C / SIGTERM that cancels the rollout's `finally` mid-teardown still frees the workspace / container / sandbox, reusing each backend's own cleanup. The hook must be sync: at interpreter shutdown the event loop and its thread-pool are gone, so async teardown raises "cannot schedule new futures". Drop the PID-tagged `reap_orphans` startup sweep. A SIGKILL/OOM runs no in-process code at all, so reclaiming it needs an external mechanism; prime sandboxes already self-terminate via their server-side max-lifetime, and the local subprocess/docker cases are out of scope. Prefix workspaces/containers/scripts with `vf-` (was `v1-`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`cleanup()` (the atexit backstop) only stopped the tunnels and left the sandbox — the costly resource — to its server-side max-lifetime. prime_sandboxes ships a sync `SandboxClient`, so delete the sandbox synchronously there as well (the async client can't run once the loop is gone). Idempotent with the async `stop` on the normal path: a second delete just 404s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The inline comments pushed two lines past the 88-col limit; moving them above the statement keeps `ruff format` happy without ruff's awkward auto-wrap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…comments - rename the module-level helpers to public `register` / `cleanup_at_exit` - trim the `_LIVE` block comment and drop the inline "no event loop" why-comments (the `cleanup` docstring already covers why teardown is sync) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikasenghaas
marked this pull request as ready for review
June 9, 2026 21:53
Contributor
ApprovabilityVerdict: Needs human review This PR adds new runtime cleanup infrastructure via atexit hooks and converts async cleanup to synchronous cleanup across all runtime types. These are meaningful runtime behavior changes affecting shutdown/interrupt handling that warrant human review. You can customize Macroscope's approvability policy. Learn more. |
pull Bot
pushed a commit
to Stars1233/verifiers
that referenced
this pull request
Jun 23, 2026
* fix(v1): reclaim orphaned subprocess workspaces
A rollout's /tmp workspace is removed in `stop()`, but a process killed mid-rollout
(SIGKILL, OOM, hard crash, interrupted teardown) never reaches it, so the workspace
leaks with no way to reclaim it — repeated runs eventually fill /tmp ("No space left
on device" at mkdtemp).
Name each workspace `/tmp/v1-<pid>-*` and, once per process on the first `start()`,
sweep `/tmp/v1-<pid>-*` whose pid is no longer alive. PID-keyed, so a concurrent live
process's workspaces are never touched; graceful per-rollout cleanup (`stop()`) is
unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(v1): atexit-based runtime teardown; drop the SIGKILL reaper
Make resource cleanup a backend-agnostic property of `Runtime`:
- a sync `cleanup()` is the teardown source of truth; the public async `stop()` runs it
off the event loop on the happy path.
- `make_runtime` registers each runtime in a WeakSet and arms one sync `atexit` hook that
calls `cleanup()` on anything still live — so a Ctrl-C / SIGTERM that cancels the
rollout's `finally` mid-teardown still frees the workspace / container / sandbox, reusing
each backend's own cleanup. The hook must be sync: at interpreter shutdown the event loop
and its thread-pool are gone, so async teardown raises "cannot schedule new futures".
Drop the PID-tagged `reap_orphans` startup sweep. A SIGKILL/OOM runs no in-process code at
all, so reclaiming it needs an external mechanism; prime sandboxes already self-terminate
via their server-side max-lifetime, and the local subprocess/docker cases are out of scope.
Prefix workspaces/containers/scripts with `vf-` (was `v1-`).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(v1): delete the prime sandbox in the sync atexit cleanup too
`cleanup()` (the atexit backstop) only stopped the tunnels and left the sandbox — the
costly resource — to its server-side max-lifetime. prime_sandboxes ships a sync
`SandboxClient`, so delete the sandbox synchronously there as well (the async client can't
run once the loop is gone). Idempotent with the async `stop` on the normal path: a second
delete just 404s.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: move teardown comments off the statement line (ruff format)
The inline comments pushed two lines past the 88-col limit; moving them above the
statement keeps `ruff format` happy without ruff's awkward auto-wrap.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(v1): public register/cleanup_at_exit, trim runtime-teardown comments
- rename the module-level helpers to public `register` / `cleanup_at_exit`
- trim the `_LIVE` block comment and drop the inline "no event loop" why-comments
(the `cleanup` docstring already covers why teardown is sync)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
A runtime provisions an external resource (a
/tmpworkspace, a docker container, a remote sandbox).stop()frees it on the normal path (it runs from the rollout'sfinally, covering success / error / cancellation), but a catchable exit (Ctrl-C / SIGTERM) can cancel thatfinallymid-teardown — and for an async teardown (a sandbox HTTP-delete,docker rm) the cancellation can cut it off, leaking the resource. Repeated leaks fill/tmp(No space left on device) or pile up sandboxes/containers.General, backend-agnostic fix in
Runtime:cleanup()— synchronous teardown, the source of truth (it must work from the atexit hook, where the event loop and its thread-pool are gone, so async teardown raisescannot schedule new futures).stop()— public async, the happy path. Defaults to runningcleanup()off the event loop; prime overrides it for the async sandbox delete + clientaclose.make_runtimeregisters each runtime in aWeakSetand arms a single syncatexithook that callscleanup()on anything still live. So a signal that cuts thefinallyshort still frees the workspace / container / sandbox — reusing each backend's own cleanup, no per-backend signal code.Per-backend
cleanup()is plain blocking work:shutil.rmtree(subprocess),subprocess.run(["docker","rm","-f"])(docker), tunnel stop + a syncSandboxClient(...).delete()(prime — so the sandbox, the costly resource, is deleted at exit too, not left to its TTL).Also renames the
v1-resource prefix tovf-.SIGKILL is intentionally out of scope
SIGKILL/OOM can't be caught — no signal handler,atexit, orfinallyruns — so the dying process can't release anything external. Reclaiming those needs a mechanism outside the process (a startup reaper, a supervisor, or a server-side TTL). Prime sandboxes still self-terminate via theirmax_lifetimeas the final net; aSIGKILLed subprocess/docker leaves its workspace/container, which is left out of scope here rather than carrying a PID-tagged reaper.Verification
Ran
gsm8k-v1(real modelarcee-ai/trinity-minivia Prime Inference) on all three runtimes, for the happy path and a mid-flight Ctrl-C, checking that no runtime resource is left behind before or after each run:/tmp/vf-*workdirsvf-*containersvf-programsandboxesThe prime Ctrl-C row is the important one: the two in-flight sandboxes were deleted on interrupt (via
stop()/ the synccleanup()at atexit), not left to theirmax_lifetime.Mechanism checks: confirmed the async teardown fails at interpreter shutdown (
cannot schedule new futures) while the synccleanup()succeeds (why the atexit path is sync), and that the prime syncSandboxClient(APIClient()).delete(...)path reaches the API.Note
Medium Risk
Touches teardown for all runtimes including Prime API deletes and Docker container removal; behavior change on interrupt paths but idempotent and best-effort.
Overview
Adds a sync teardown path so runtimes still free workspaces, Docker containers, and Prime sandboxes when Ctrl-C/SIGTERM cancels the rollout
finallybefore asyncstop()finishes.The
Runtimecontract gainscleanup()as the idempotent, blocking teardown API; defaultstop()delegates to it viaasyncio.to_thread.make_runtimeregisters each instance in aWeakSetand arms a one-timeatexithook that callscleanup()on anything still live. Subprocess, Docker, and Prime each implementcleanup()(e.g.shutil.rmtree, syncdocker rm, sync sandbox delete + tunnel stop); Prime keeps asyncstop()for the normal API delete path.Also renames runtime resource prefixes from
v1-tovf-(workdirs, containers, sandboxes, script cache paths).Reviewed by Cursor Bugbot for commit 44edc4b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Free runtime resources on process exit via synchronous
cleanup()andatexitregistrationcleanup()method to theRuntimebase class and overrides it inDockerRuntime,PrimeRuntime, andSubprocessRuntimeto tear down containers, sandboxes, and processes without requiring an event loop.WeakSet-based registry in base.py that lazily registers anatexithook; all runtimes created viamake_runtimeare registered automatically.stop()now delegates tocleanup()viaasyncio.to_thread, unifying teardown through the sync path.v1-tovf-across Docker containers, Prime sandboxes, subprocess workspaces, and script paths under/tmp.v1-prefixes will not be matched or cleaned up by the new code; script paths under/tmpalso change, invalidating uv cache keys.Macroscope summarized 44edc4b.