Skip to content

pool: don't lose connections handed to a cancelled waiter - #1085

Open
Fizzadar wants to merge 1 commit into
mainfrom
fix/pool-lost-connection-race
Open

pool: don't lose connections handed to a cancelled waiter#1085
Fizzadar wants to merge 1 commit into
mainfrom
fix/pool-lost-connection-race

Conversation

@Fizzadar

@Fizzadar Fizzadar commented Jul 27, 2026

Copy link
Copy Markdown
Member

Problem

DC.acquire's third case (wait for a free connection) can permanently lose a live connection.

reqMap.transfer removes the waiting request's key from the map, unlocks r.mux, and only then sends the connection on the request's buffered channel. A waiter whose context is cancelled inside that window calls reqMap.delete (now a no-op, the key is gone) and then does a non-blocking receive, which finds the channel still empty. The connection is then sent into a channel nobody will ever read:

  • it never goes back to c.free,
  • it is still counted in c.total,
  • it stays alive, so it never triggers c.dead / c.stuck and the pool cannot recover.

Sub-DC pools are created with max = 1 (Client.invokeSub), so one lost connection wedges every subsequent acquire on that datacenter until the process restarts.

DC.acquire's third case waits for a free connection and, if its context is
cancelled, deletes its request and does a non-blocking receive on the request
channel. reqMap.transfer removes the request key, unlocks, and only then sends
the connection, so a waiter that cancels inside that window misses it: the
connection stays in the buffered channel nobody reads, never returns to c.free,
and is still counted in c.total. It also never dies, so c.stuck never fires and
the pool cannot recover.

Sub-DC pools are created with max=1, so a single lost connection stops all media
downloads on that datacenter until the process restarts. Seen on a bridge where
every direct download failed with "acquire connection: context canceled" for 17
hours while the connection itself kept running.

Make reqMap.delete report whether the request was still pending, and when
transfer already claimed it, wait for the connection and hand it back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Fizzadar
Fizzadar marked this pull request as ready for review July 27, 2026 12:45
@Fizzadar
Fizzadar requested a review from tulir July 27, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant