Skip to content

Commit 1081b0b

Browse files
authored
Don't crash when hitting long backoffs. (swift-server#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‎

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

4848
letstart=Double(TimeAmount.milliseconds(100).nanoseconds)
49-
letbackoffNanoseconds=Int64(start * pow(1.25,Double(attempts -1)))
49+
letbackoffNanosecondsDouble= start * pow(1.25,Double(attempts -1))
5050

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

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

‎Tests/AsyncHTTPClientTests/HTTPConnectionPoolTests+XCTest.swift‎

Lines changed: 1 addition & 0 deletions
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‎

Lines changed: 26 additions & 0 deletions
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+
varbackoff=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+
varattempt=1
466+
while backoff <(.seconds(60)-.milliseconds(1800)){
467+
attempt +=1
468+
letnewBackoff=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+
foroffsetin0..<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
classTestDelegate:HTTPConnectionPoolDelegate{

0 commit comments

Comments
(0)