Commit bba3007
committed
fix(pooling): fix connection leak in shutdown() during KILL/reconnect cycle
The previous isShutdown early-return made shutdown() a no-op on the
second invocation, which leaked the new connection opened by a
reconnect after a KILL.
Sequence with the bug:
1. Heartbeat fires KILL -> shutdown() -> client.close() (closes old
connection), isShutdown=true, emit onConnectionClosed
2. connectAfter() schedules a reconnect (1s later)
3. Reconnect fires -> internalConnect() opens a new channel on the
same ConnectionImpl (state CLOSED -> CONNECTING -> READY)
4. The new connection's auth/ping times out (pipe locked in heartbeat
tests) -> handleConnectError -> shutdown() is now a no-op, so
client.close() is NOT called on the new connection
5. connectAfter() schedules ANOTHER reconnect, leaving the new
connection open and leaking
This caused the testReconnectsAfterInvalidationAndKill test to time
out at 60s because active connections to node-a never reached 0
after KILL, and the test was waiting for that condition.
Fix: keep the client.close() call (and stopHeartbeat/connectFuture
cleanup) on every shutdown() invocation, but guard only the
onConnectionClosed emit with the isShutdown flag. This way the
client is always closed (idempotently, since closeChannel is a no-op
if state is already CLOSED/CLOSING), but the pool's
onConnectionClosed listener fires only once per PoolEntry, preventing
double-decrement of the unavailable counter.
Verified:
- ConnectionPoolReconnectsTest: 5/5 runs pass
- ConnectionPoolTest: 5/5 runs pass (12 tests each)
- All 24 unit tests pass
ConnectionPoolHeartbeatTest is still flaky (3-5/10 pass rate) due to
a separate test-infrastructure issue: the test asserts on tarantool's
box.stat.net().CONNECTIONS.current counter, which is updated
asynchronously and is often 0-6 below the expected count when 20+
connections are created in a tight burst. This is a test-side timing
race, not a PoolEntry bug, and cannot be fixed without modifying the
test to wait for the counter to stabilize.1 parent ce98b46 commit bba3007
1 file changed
Lines changed: 3 additions & 4 deletions
Lines changed: 3 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
352 | 352 | | |
353 | 353 | | |
354 | 354 | | |
355 | | - | |
356 | | - | |
357 | | - | |
358 | 355 | | |
359 | 356 | | |
360 | 357 | | |
361 | 358 | | |
362 | 359 | | |
363 | 360 | | |
364 | 361 | | |
365 | | - | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
366 | 365 | | |
367 | 366 | | |
368 | 367 | | |
| |||
0 commit comments