Skip to content

Commit 1081b0b

Browse files
authored
Don't crash when hitting long backoffs. (#458)
Motivation: If we backoff sufficiently far we can overflow Int64, which will cause us to crash. Modifications: Clamp the backoff value before we convert to Int64. Results: No crashes!
1 parent d5bd8d6 commit 1081b0b

File tree

3 files changed

+32
-2
lines changed

3 files changed

+32
-2
lines changed

Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+Backoff.swift

+5-2
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,12 @@ extension HTTPConnectionPool {
4646
// - 29 failed attempts: ~60s (max out)
4747

4848
let start = Double(TimeAmount.milliseconds(100).nanoseconds)
49-
let backoffNanoseconds = Int64(start * pow(1.25, Double(attempts - 1)))
49+
let backoffNanosecondsDouble = start * pow(1.25, Double(attempts - 1))
5050

51-
let backoff: TimeAmount = min(.nanoseconds(backoffNanoseconds), .seconds(60))
51+
// Cap to 60s _before_ we convert to Int64, to avoid trapping in the Int64 initializer.
52+
let backoffNanoseconds = Int64(min(backoffNanosecondsDouble, Double(TimeAmount.seconds(60).nanoseconds)))
53+
54+
let backoff = TimeAmount.nanoseconds(backoffNanoseconds)
5255

5356
// Calculate a 3% jitter range
5457
let jitterRange = (backoff.nanoseconds / 100) * 3

Tests/AsyncHTTPClientTests/HTTPConnectionPoolTests+XCTest.swift

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ extension HTTPConnectionPoolTests {
3333
("testConnectionCreationIsRetriedUntilRequestIsCancelled", testConnectionCreationIsRetriedUntilRequestIsCancelled),
3434
("testConnectionShutdownIsCalledOnActiveConnections", testConnectionShutdownIsCalledOnActiveConnections),
3535
("testConnectionPoolStressResistanceHTTP1", testConnectionPoolStressResistanceHTTP1),
36+
("testBackoffBehavesSensibly", testBackoffBehavesSensibly),
3637
]
3738
}
3839
}

Tests/AsyncHTTPClientTests/HTTPConnectionPoolTests.swift

+26
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,32 @@ class HTTPConnectionPoolTests: XCTestCase {
453453
pool.shutdown()
454454
XCTAssertNoThrow(try poolDelegate.future.wait())
455455
}
456+
457+
func testBackoffBehavesSensibly() throws {
458+
var backoff = HTTPConnectionPool.calculateBackoff(failedAttempt: 1)
459+
460+
// The value should be 100ms±3ms
461+
XCTAssertLessThanOrEqual((backoff - .milliseconds(100)).nanoseconds.magnitude, TimeAmount.milliseconds(3).nanoseconds.magnitude)
462+
463+
// Should always increase
464+
// We stop when we get within the jitter of 60s, which is 1.8s
465+
var attempt = 1
466+
while backoff < (.seconds(60) - .milliseconds(1800)) {
467+
attempt += 1
468+
let newBackoff = HTTPConnectionPool.calculateBackoff(failedAttempt: attempt)
469+
470+
XCTAssertGreaterThan(newBackoff, backoff)
471+
backoff = newBackoff
472+
}
473+
474+
// Ok, now we should be able to do a hundred increments, and always hit 60s, plus or minus 1.8s of jitter.
475+
for offset in 0..<100 {
476+
XCTAssertLessThanOrEqual(
477+
(HTTPConnectionPool.calculateBackoff(failedAttempt: attempt + offset) - .seconds(60)).nanoseconds.magnitude,
478+
TimeAmount.milliseconds(1800).nanoseconds.magnitude
479+
)
480+
}
481+
}
456482
}
457483

458484
class TestDelegate: HTTPConnectionPoolDelegate {

0 commit comments

Comments
 (0)