Skip to content

Commit 1120541

Browse files
authored
Fix crash when writablity becomes false and races against finishing the http request (#768)
### Motivation If the channel's writability changed to false just before we finished a request, we currently run into a precondition. ### Changes - Remove the precondition and handle the case appropiatly ### Result A crash less.
1 parent 776a1c2 commit 1120541

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ClientChannelHandler.swift

+2-1
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,8 @@ struct IdleWriteStateMachine {
665665
self.state = .requestEndSent
666666
return .clearIdleWriteTimeoutTimer
667667
case .waitingForWritabilityEnabled:
668-
preconditionFailure("If the channel is not writable, we can't have sent the request end.")
668+
self.state = .requestEndSent
669+
return .none
669670
case .requestEndSent:
670671
return .none
671672
}

Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests.swift

+50
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,56 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
376376
}
377377
}
378378

379+
func testIdleWriteTimeoutRaceToEnd() {
380+
let embedded = EmbeddedChannel()
381+
var maybeTestUtils: HTTP1TestTools?
382+
XCTAssertNoThrow(maybeTestUtils = try embedded.setupHTTP1Connection())
383+
guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
384+
385+
var maybeRequest: HTTPClient.Request?
386+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream { _ in
387+
// Advance time by more than the idle write timeout (that's 1 millisecond) to trigger the timeout.
388+
let scheduled = embedded.embeddedEventLoop.flatScheduleTask(in: .milliseconds(2)) {
389+
embedded.embeddedEventLoop.makeSucceededVoidFuture()
390+
}
391+
return scheduled.futureResult
392+
}))
393+
394+
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }
395+
396+
let delegate = ResponseAccumulator(request: request)
397+
var maybeRequestBag: RequestBag<ResponseAccumulator>?
398+
XCTAssertNoThrow(maybeRequestBag = try RequestBag(
399+
request: request,
400+
eventLoopPreference: .delegate(on: embedded.eventLoop),
401+
task: .init(eventLoop: embedded.eventLoop, logger: testUtils.logger),
402+
redirectHandler: nil,
403+
connectionDeadline: .now() + .seconds(30),
404+
requestOptions: .forTests(idleWriteTimeout: .milliseconds(5)),
405+
delegate: delegate
406+
))
407+
guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
408+
409+
embedded.isWritable = true
410+
embedded.pipeline.fireChannelWritabilityChanged()
411+
testUtils.connection.executeRequest(requestBag)
412+
let expectedHeaders: HTTPHeaders = ["host": "localhost", "Transfer-Encoding": "chunked"]
413+
XCTAssertEqual(
414+
try embedded.readOutbound(as: HTTPClientRequestPart.self),
415+
.head(HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: expectedHeaders))
416+
)
417+
418+
// change the writability to false.
419+
embedded.isWritable = false
420+
embedded.pipeline.fireChannelWritabilityChanged()
421+
embedded.embeddedEventLoop.run()
422+
423+
// let the writer, write an end (while writability is false)
424+
embedded.embeddedEventLoop.advanceTime(by: .milliseconds(2))
425+
426+
XCTAssertEqual(try embedded.readOutbound(as: HTTPClientRequestPart.self), .end(nil))
427+
}
428+
379429
func testIdleWriteTimeoutWritabilityChanged() {
380430
let embedded = EmbeddedChannel()
381431
let testWriter = TestBackpressureWriter(eventLoop: embedded.eventLoop, parts: 5)

0 commit comments

Comments
 (0)