fix(runtime): close the docker client on session cleanup#787
Merged
Conversation
cleanup() removed the container but never closed the Docker SDK client, so docker-py's requests/urllib3 connection pool was left for the interpreter to finalize at GC/shutdown. urllib3 response finalizers then raise "I/O operation on closed file" straight to stderr, bypassing the logger. A run that dies mid-exec orphans a batch of responses at once and floods the terminal with identical tracebacks. Close the client after delete(), best-effort so cleanup never blocks the next scan. The client is per-scan (backends.py builds a fresh docker.from_env() per session), so this cannot affect a concurrent scan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Greptile SummaryThis PR adds deterministic Docker client cleanup for completed sandbox sessions. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (1): Last reviewed commit: "fix(runtime): close the docker client on..." | Re-trigger Greptile |
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.
Problem
session_manager.cleanup()removes the sandbox container viaclient.delete()but never closes the Docker SDK client. docker-py talks to the daemon overrequests/urllib3, so the connection pool is left for the interpreter to finalize at GC/shutdown. urllib3's response finalizers then raise:These go straight to stderr via
sys.unraisablehook, bypassing the logger entirely (0 occurrences instrix.log), which is why they show up as unexplained terminal spam. A run that dies mid-exec orphans a batch of responses at once — an observed failure printed ~25 identical tracebacks back to back.Fix
Close the client after
delete(), best-effort so cleanup never blocks the next scan.backends.py:50builds a freshdocker.from_env()per session, so closing one scan's client cannot affect a concurrent scan.getattrguard keeps this resilient if the SDK renames the attribute.Scope / honesty
These finalizer errors are cosmetic — an exception raised in a finalizer is caught and printed by CPython; it cannot propagate or affect a run's outcome. This is noise reduction plus correct deterministic teardown, not a behavior fix.
I was not able to reproduce the finalizer noise in a standalone harness (tried clean execs and abandoned streaming execs with the container force-removed underneath), so there is no before/after proof that it eliminates every occurrence. The change is correct-by-construction — the tracebacks are in urllib3, which reaches Docker via requests/docker-py (LiteLLM/OpenAI use httpx, so they're excluded), and
docker_client.close()→api.close()tears down exactly that pool. If any responses originate from the SDK's exec transport rather than docker-py's pool, those would need a separate fix.Follow-up (not in this PR)
The same teardown path has a more serious gap: when
delete()raises (e.g.409 Conflicton an already-stopped container), the container is left un-reaped. An orphaned sandbox kept writing to its unbounded log and reached 147 GB, filling the host disk and taking the daemon down. #785 caps the log; force-reaping ondelete()failure is still open and worth its own change.Verification
ruff-lint,ruff-format,mypy,bandit,pyupgradeall pass (full pre-commit suite).client.close()/client.api.close()exist in the pinned docker-py (7.1.0).🤖 Generated with Claude Code