Skip to content

Commit a5c85d2

Browse files
authored
Fixes crash: Wrong expectation – request queue may not be empty when keep-alive returns (#633)
1 parent cc0201c commit a5c85d2

File tree

2 files changed

+76
-1
lines changed

2 files changed

+76
-1
lines changed

Sources/ConnectionPoolModule/PoolStateMachine.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,10 @@ struct PoolStateMachine<
558558
@inlinable
559559
mutating func connectionKeepAliveTimerTriggered(_ connectionID: ConnectionID) -> Action {
560560
precondition(self.configuration.keepAliveDuration != nil)
561-
precondition(self.requestQueue.isEmpty)
561+
// Removed: precondition(self.requestQueue.isEmpty)
562+
// A lease request may have been queued after this keep-alive timer was scheduled
563+
// (e.g. PG restart caused connections to close while new requests arrived).
564+
// This mirrors the fix in connectionIdleTimerTriggered (PR #627).
562565

563566
guard let keepAliveAction = self.connections.keepAliveIfIdle(connectionID) else {
564567
return .none()

Tests/ConnectionPoolModuleTests/PoolStateMachineTests.swift

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,4 +977,76 @@ typealias TestPoolStateMachine = PoolStateMachine<
977977
return
978978
}
979979
}
980+
981+
// Regression test: keep-alive timer fires while requestQueue is non-empty.
982+
// This reproduces a crash (precondition failure at PoolStateMachine.swift:561)
983+
// when PG restarts: connection creation fails, new lease requests get queued,
984+
// but keep-alive timers on surviving idle connections are still armed.
985+
// Mirrors the fix in connectionIdleTimerTriggered (vapor/postgres-nio PR #627).
986+
@available(macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0, *)
987+
@Test func testKeepAliveTimerWithQueuedRequests() {
988+
var configuration = PoolConfiguration()
989+
configuration.minimumConnectionCount = 2
990+
configuration.maximumConnectionSoftLimit = 2
991+
configuration.maximumConnectionHardLimit = 2
992+
configuration.keepAliveDuration = .seconds(10)
993+
994+
var stateMachine = TestPoolStateMachine(
995+
configuration: configuration,
996+
generator: .init(),
997+
timerCancellationTokenType: MockTimerCancellationToken.self,
998+
clock: MockClock()
999+
)
1000+
1001+
// 1. Create two connections (the pool minimum)
1002+
let requests = stateMachine.refillConnections()
1003+
#expect(requests.count == 2)
1004+
1005+
let connection0 = MockConnection(id: 0)
1006+
let connection1 = MockConnection(id: 1)
1007+
1008+
let created0 = stateMachine.connectionEstablished(connection0, maxStreams: 1)
1009+
let timer0 = TestPoolStateMachine.Timer(.init(timerID: 0, connectionID: 0, usecase: .keepAlive), duration: .seconds(10))
1010+
let cancel0 = MockTimerCancellationToken(timer0)
1011+
#expect(created0.connection == .scheduleTimers([timer0]))
1012+
#expect(stateMachine.timerScheduled(timer0, cancelContinuation: cancel0) == .none)
1013+
1014+
let created1 = stateMachine.connectionEstablished(connection1, maxStreams: 1)
1015+
let timer1 = TestPoolStateMachine.Timer(.init(timerID: 0, connectionID: 1, usecase: .keepAlive), duration: .seconds(10))
1016+
let cancel1 = MockTimerCancellationToken(timer1)
1017+
#expect(created1.connection == .scheduleTimers([timer1]))
1018+
#expect(stateMachine.timerScheduled(timer1, cancelContinuation: cancel1) == .none)
1019+
1020+
// Both connections are idle with keep-alive timers armed
1021+
#expect(stateMachine.connections.stats.idle == 2)
1022+
1023+
// 2. Close connection 1 — pool will try to create a replacement
1024+
connection1.close()
1025+
let closedAction = stateMachine.connectionClosed(connection1)
1026+
// Pool should request a new connection to maintain minimum.
1027+
// Only connection 1's keep-alive timer is cancelled (connection 0 is still alive).
1028+
#expect(closedAction.connection == .makeConnection(.init(connectionID: 2), [cancel1]))
1029+
1030+
// 3. The replacement connection (id=2) fails to connect (PG is down)
1031+
let failAction = stateMachine.connectionEstablishFailed(
1032+
SomeError(),
1033+
for: .init(connectionID: 2)
1034+
)
1035+
// Pool enters connectionCreationFailing state and schedules a backoff timer
1036+
#expect(failAction.request == .none)
1037+
1038+
// 4. A lease request arrives — gets queued because pool is in connectionCreationFailing
1039+
let request = MockRequest(connectionType: MockConnection.self)
1040+
let leaseAction = stateMachine.leaseConnection(request)
1041+
#expect(leaseAction.request == .none) // queued, not served
1042+
#expect(leaseAction.connection == .none)
1043+
1044+
// 5. Keep-alive timer fires on connection 0 (still idle).
1045+
// BUG: precondition(self.requestQueue.isEmpty) crashes here.
1046+
// FIX: the guard on keepAliveIfIdle handles this safely.
1047+
let keepAliveAction = stateMachine.connectionKeepAliveTimerTriggered(connection0.id)
1048+
#expect(keepAliveAction.connection == .runKeepAlive(connection0, cancel0))
1049+
}
9801050
}
1051+
1052+
struct SomeError: Error {}

0 commit comments

Comments
 (0)