mirror of
https://github.com/cloudflare/pingora.git
synced 2026-05-15 09:50:42 +00:00
zaidoon
0f1248114f
ConnectionPool's internal HashMap never removed PoolNode entries after all their connections were drained. Each empty PoolNode retains ~1 KB (mainly from the pre-allocated ArrayQueue), so workloads connecting to many unique upstreams over time would see unbounded memory growth. Root cause: there was exactly one code path inserting keys into the pool HashMap (get_pool_node → pool.insert) and zero code paths removing them. The fix adds inline cleanup at each point where a node can become empty: - get(): after taking the last connection (important because idle_poll/ idle_timeout exit via watch_use and never call pop_closed) - pop_evicted(): after LRU eviction removes a connection - pop_closed(): inherits cleanup from the above paths Cleanup uses a double-check pattern: callers first check is_empty() (a cheap atomic load on the hot queue that short-circuits in the common case), then call try_remove_empty_node() which re-verifies under the write lock to avoid removing a node that was concurrently repopulated. The same bug existed in InUsePool (pingora-core h2 connector), which is also fixed here with the same pattern applied to get() and release(). Hot-path cost: one additional atomic load (~1 ns) when connections remain in the node. The write lock is only acquired on the cold path when a node actually empties, matching the cleanup strategy used by hyper-util and Go's net/http.