Skip to content

Commit

Permalink
Don't crash when hitting long backoffs. (#458)
Browse files Browse the repository at this point in the history
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!
  • Loading branch information
Lukasa authored Oct 13, 2021
1 parent d5bd8d6 commit 1081b0b
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,12 @@ extension HTTPConnectionPool {
// - 29 failed attempts: ~60s (max out)

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

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

let backoff = TimeAmount.nanoseconds(backoffNanoseconds)

// Calculate a 3% jitter range
let jitterRange = (backoff.nanoseconds / 100) * 3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ extension HTTPConnectionPoolTests {
("testConnectionCreationIsRetriedUntilRequestIsCancelled", testConnectionCreationIsRetriedUntilRequestIsCancelled),
("testConnectionShutdownIsCalledOnActiveConnections", testConnectionShutdownIsCalledOnActiveConnections),
("testConnectionPoolStressResistanceHTTP1", testConnectionPoolStressResistanceHTTP1),
("testBackoffBehavesSensibly", testBackoffBehavesSensibly),
]
}
}
26 changes: 26 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTPConnectionPoolTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,32 @@ class HTTPConnectionPoolTests: XCTestCase {
pool.shutdown()
XCTAssertNoThrow(try poolDelegate.future.wait())
}

func testBackoffBehavesSensibly() throws {
var backoff = HTTPConnectionPool.calculateBackoff(failedAttempt: 1)

// The value should be 100ms±3ms
XCTAssertLessThanOrEqual((backoff - .milliseconds(100)).nanoseconds.magnitude, TimeAmount.milliseconds(3).nanoseconds.magnitude)

// Should always increase
// We stop when we get within the jitter of 60s, which is 1.8s
var attempt = 1
while backoff < (.seconds(60) - .milliseconds(1800)) {
attempt += 1
let newBackoff = HTTPConnectionPool.calculateBackoff(failedAttempt: attempt)

XCTAssertGreaterThan(newBackoff, backoff)
backoff = newBackoff
}

// Ok, now we should be able to do a hundred increments, and always hit 60s, plus or minus 1.8s of jitter.
for offset in 0..<100 {
XCTAssertLessThanOrEqual(
(HTTPConnectionPool.calculateBackoff(failedAttempt: attempt + offset) - .seconds(60)).nanoseconds.magnitude,
TimeAmount.milliseconds(1800).nanoseconds.magnitude
)
}
}
}

class TestDelegate: HTTPConnectionPoolDelegate {
Expand Down

0 comments on commit 1081b0b

Please sign in to comment.