From bbf9c06bdb54735e32f744cd071c1c113faac815 Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Mon, 27 Jul 2026 13:14:52 +0100 Subject: [PATCH] pool: don't lose connections handed to a cancelled waiter 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) --- pkg/gotd/pool/pool.go | 17 +++++++---------- pkg/gotd/pool/req_map.go | 6 +++++- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/gotd/pool/pool.go b/pkg/gotd/pool/pool.go index 449c856f2..437b71e78 100644 --- a/pkg/gotd/pool/pool.go +++ b/pkg/gotd/pool/pool.go @@ -207,11 +207,9 @@ retry: case <-c.stuck.Ready(): c.log.Debug("Some connection dead, try to create new connection, cancel waiting") - c.freeReq.delete(key) - select { - default: - case conn, ok := <-ch: - if ok && conn != nil { + if !c.freeReq.delete(key) { + // transfer took the request, so a connection is on its way and must not be dropped. + if conn, ok := <-ch; ok && conn != nil { return conn, nil } } @@ -224,11 +222,10 @@ retry: } // Executed only if at least one of context is Done. - c.freeReq.delete(key) - select { - default: - case conn, ok := <-ch: - if ok && conn != nil { + if !c.freeReq.delete(key) { + // transfer took the request, so a connection is on its way and must be given back to the + // pool instead of being left in the channel. + if conn, ok := <-ch; ok && conn != nil { c.release(conn) } } diff --git a/pkg/gotd/pool/req_map.go b/pkg/gotd/pool/req_map.go index 38e6af10a..f5592becb 100644 --- a/pkg/gotd/pool/req_map.go +++ b/pkg/gotd/pool/req_map.go @@ -56,8 +56,12 @@ func (r *reqMap) transfer(c *poolConn) bool { return true } -func (r *reqMap) delete(key reqKey) { +// delete removes the pending request. It returns false if transfer already claimed the request, +// which means a connection is about to be sent to the request channel. +func (r *reqMap) delete(key reqKey) bool { r.mux.Lock() + _, ok := r.m[key] delete(r.m, key) r.mux.Unlock() + return ok }