Skip to content

Share Prime sandbox clients across concurrent runtimes - #2411

Open
xeophon wants to merge 3 commits into
mainfrom
fix/prime-sandbox-client-pool
Open

Share Prime sandbox clients across concurrent runtimes#2411
xeophon wants to merge 3 commits into
mainfrom
fix/prime-sandbox-client-pool

Conversation

@xeophon

@xeophon xeophon commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overview

Share one Prime Sandboxes async client across active Prime runtimes on the same event loop. This lets the SDK coalesce concurrent sandbox and background-job status polls into its bounded batch requests.

Details

  • Add an event-loop-scoped, reference-counted PrimeClientPool.
  • Lease the shared client during runtime startup while preserving each sandbox's independent lifecycle.
  • Release the lease during teardown and close the client when the final runtime exits.

Note

Medium Risk
Changes shared client lifecycle and teardown for remote sandboxes. Incorrect lease counting or loop-keyed global state could close a client too early or leak it.

Overview
Shares one Prime AsyncSandboxClient across concurrent PrimeRuntimes on the same event loop so the SDK can batch status polls.

Adds a reference-counted PrimeClientPool. start leases the loop’s client instead of creating a new one; teardown still deletes each sandbox independently, then drops the lease and acloses only when the last runtime on that loop exits.

Reviewed by Cursor Bugbot for commit 7661ba7. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Share AsyncSandboxClient across PrimeRuntime instances per event loop

  • Introduces PrimeClientPool and a client_pools dict mapping asyncio event loops to shared AsyncSandboxClient instances and lease counts.
  • PrimeRuntime.start now reuses an existing client for the current event loop or creates a new one, incrementing the pool's lease counter.
  • PrimeRuntime.teardown decrements the lease counter and only calls client.aclose() when the last runtime on that loop tears down.
  • Risk: PrimeRuntime.teardown no longer unconditionally closes the client; incorrect lease tracking could leak or prematurely close AsyncSandboxClient instances.

Macroscope summarized 7661ba7.

Comment thread verifiers/v1/runtimes/prime.py Outdated
if clientPool is None:
clientPool = PrimeClientPool(loop=loop, client=AsyncSandboxClient())
clientPools[loop] = clientPool
clientPool.users += 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium runtimes/prime.py:176

A provisioning failure leaves clientPool.users incremented permanently, so after the other runtimes tear down the shared AsyncSandboxClient remains in clientPools and its HTTP resources are never closed. _serve only registers runtime.stop after await runtime.start() succeeds, so this failed start path cannot reach teardown(); roll back the pool lease in the start exception path and close/remove the pool when that rollback reaches zero.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/runtimes/prime.py around line 176:

A provisioning failure leaves `clientPool.users` incremented permanently, so after the other runtimes tear down the shared `AsyncSandboxClient` remains in `clientPools` and its HTTP resources are never closed. `_serve` only registers `runtime.stop` after `await runtime.start()` succeeds, so this failed `start` path cannot reach `teardown()`; roll back the pool lease in the `start` exception path and close/remove the pool when that rollback reaches zero.

@macroscopeapp

macroscopeapp Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces client pooling with lease counting for Prime sandbox clients - a significant runtime behavior change to resource management. Additionally, unresolved comments identify a resource leak when start() fails (lease incremented but never decremented on failure path). Human review recommended for both the infrastructure change and the identified bug.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7661ba728d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pool = client_pools.get(loop)
if pool is None:
pool = client_pools[loop] = PrimeClientPool(client=AsyncSandboxClient())
pool.leases += 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Release the pooled lease when startup fails

When a non-colocated MCP server uses Prime and runtime.start() raises, _serve registers runtime.stop only after startup returns (verifiers/v1/mcp/launch.py:372-374), so this increment is never balanced. The module-level pool then strongly retains the event loop and client with a permanently positive lease count; subsequent runtimes reuse that poisoned pool, and their final teardown never removes or closes it. Release the lease on failed startup or ensure teardown is registered before awaiting start().

Useful? React with 👍 / 👎.

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