Skip to content

fix: wait for in-flight acquisitions when closing connection pools - #8244

Open
maximilliangrand wants to merge 1 commit into
livekit:mainfrom
maximilliangrand:fix/connection-pool-close-handshake
Open

maximilliangrand wants to merge 1 commit into
livekit:mainfrom
maximilliangrand:fix/connection-pool-close-handshake

Conversation

@maximilliangrand

@maximilliangrand maximilliangrand commented Oct 29, 2026 •

Copy link
Copy Markdown

ConnectionPool.aclose() can return while an ordinary get() is still opening a connection. The close invalidates the in-flight attempt, so _connect() closes that result and retries after shutdown has already returned, leaving the replacement connection open. An acquisition draining a retired connection has the same race.

Serialize invalidation and draining in aclose() with the existing connection lock. This waits for the acquisition to finish before closing its connection. Prewarm cancellation still happens before taking the lock, and the pool can still be reused after closing.

The regressions cover blocked handshakes and blocked drains, exact-once closure, reuse, and cancellation of an in-flight prewarm. Both acquisition cases fail before the fix; all 19 pool tests pass with it. A separate loopback TCP reproduction leaves one live socket before the fix and none afterward.

Validation:

  • On base 049aea8, uv run --no-sync pytest tests/test_connection_pool.py --unit -q: 19 passed.
  • make check passed: Ruff formatting/lint and mypy across all 737 source files. Local commands used the checkout's core/plugin roots in PYTHONPATH to work around an editable-install import issue on macOS.
  • Independent review and four additional cancellation/concurrent-close probes passed.

The full local unit/audio-EOT suite was not repeated after rebasing. The earlier Python 3.14 run passed 6,096 tests but encountered twelve NumPy buffer-resize failures and nine missing-livekit-server setup errors, reproduced on the unchanged base, plus one audio-decoder cancellation failure that passed in isolation on both trees. Fresh PR CI will validate the updated head.

No live provider credentials were used. Implementation and independent review were AI-assisted.

@maximilliangrand
maximilliangrand requested a review from a team as a code owner October 29, 2026 09:03

@devin-ai-integration devin-ai-integration Bot left a comment

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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

This branch has not been deployed

No deployments
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.

2 participants