Skip to content

fix(v1): guard the pool health reply so a departed client can't crash the pool#2080

Open
dumko2001 wants to merge 1 commit into
PrimeIntellect-ai:mainfrom
dumko2001:fix/v1-pool-health-send-guard
Open

fix(v1): guard the pool health reply so a departed client can't crash the pool#2080
dumko2001 wants to merge 1 commit into
PrimeIntellect-ai:mainfrom
dumko2001:fix/v1-pool-health-send-guard

Conversation

@dumko2001

@dumko2001 dumko2001 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

TL;DR — under ROUTER_MANDATORY=1, one client abandoning a health poll crashes the entire env-server pool (all workers, all in-flight rollouts). Three-line fix.

Repro

Client calls .health() on a short timeout, times out, and closes its DEALER socket in the gap between the broker reading the request (pool.py:196) and sending the reply (pool.py:199). The ROUTER has already dropped that identity, so the send raises zmq.ZMQError.

Impact

run()'s only handler is except (asyncio.CancelledError, KeyboardInterrupt) (pool.py:239), so the ZMQError escapes the loop. It unwinds into finally: self._shutdown() (pool.py:242), which calls w["process"].terminate() on every worker (pool.py:249). The whole pool goes down, killing every concurrent rollout, not just the health request that triggered it.

Fix

# before
if method == b"health":
    await self.frontend.send_multipart([client_id, request_id, _HEALTH])

# after
if method == b"health":
    with contextlib.suppress(zmq.ZMQError):
        await self.frontend.send_multipart([client_id, request_id, _HEALTH])

The worker-reply send two branches down already uses this exact guard (pool.py:235); the health send was the one path missing it. Live clients are unaffected, since the suppress only swallows a send to a peer that's already gone. Happy to add a regression test that drops a client mid-health poll.

Reopens #2071.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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