From 11205411bb60612f0a1a04f733fa71b4fb864ab9 Mon Sep 17 00:00:00 2001
From: Fabian Fett <fabianfett@apple.com>
Date: Tue, 3 Sep 2024 13:54:44 +0200
Subject: [PATCH] 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.
---
 .../HTTP1/HTTP1ClientChannelHandler.swift     |  3 +-
 .../HTTP1ClientChannelHandlerTests.swift      | 50 +++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ClientChannelHandler.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ClientChannelHandler.swift
index 04de8b352..86d2547ef 100644
--- a/Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ClientChannelHandler.swift
+++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ClientChannelHandler.swift
@@ -665,7 +665,8 @@ struct IdleWriteStateMachine {
             self.state = .requestEndSent
             return .clearIdleWriteTimeoutTimer
         case .waitingForWritabilityEnabled:
-            preconditionFailure("If the channel is not writable, we can't have sent the request end.")
+            self.state = .requestEndSent
+            return .none
         case .requestEndSent:
             return .none
         }
diff --git a/Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests.swift b/Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests.swift
index f4f2d67f8..c91db94b3 100644
--- a/Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests.swift
@@ -376,6 +376,56 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
         }
     }
 
+    func testIdleWriteTimeoutRaceToEnd() {
+        let embedded = EmbeddedChannel()
+        var maybeTestUtils: HTTP1TestTools?
+        XCTAssertNoThrow(maybeTestUtils = try embedded.setupHTTP1Connection())
+        guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
+
+        var maybeRequest: HTTPClient.Request?
+        XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream { _ in
+            // Advance time by more than the idle write timeout (that's 1 millisecond) to trigger the timeout.
+            let scheduled = embedded.embeddedEventLoop.flatScheduleTask(in: .milliseconds(2)) {
+                embedded.embeddedEventLoop.makeSucceededVoidFuture()
+            }
+            return scheduled.futureResult
+        }))
+
+        guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }
+
+        let delegate = ResponseAccumulator(request: request)
+        var maybeRequestBag: RequestBag<ResponseAccumulator>?
+        XCTAssertNoThrow(maybeRequestBag = try RequestBag(
+            request: request,
+            eventLoopPreference: .delegate(on: embedded.eventLoop),
+            task: .init(eventLoop: embedded.eventLoop, logger: testUtils.logger),
+            redirectHandler: nil,
+            connectionDeadline: .now() + .seconds(30),
+            requestOptions: .forTests(idleWriteTimeout: .milliseconds(5)),
+            delegate: delegate
+        ))
+        guard let requestBag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag") }
+
+        embedded.isWritable = true
+        embedded.pipeline.fireChannelWritabilityChanged()
+        testUtils.connection.executeRequest(requestBag)
+        let expectedHeaders: HTTPHeaders = ["host": "localhost", "Transfer-Encoding": "chunked"]
+        XCTAssertEqual(
+            try embedded.readOutbound(as: HTTPClientRequestPart.self),
+            .head(HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: expectedHeaders))
+        )
+
+        // change the writability to false.
+        embedded.isWritable = false
+        embedded.pipeline.fireChannelWritabilityChanged()
+        embedded.embeddedEventLoop.run()
+
+        // let the writer, write an end (while writability is false)
+        embedded.embeddedEventLoop.advanceTime(by: .milliseconds(2))
+
+        XCTAssertEqual(try embedded.readOutbound(as: HTTPClientRequestPart.self), .end(nil))
+    }
+
     func testIdleWriteTimeoutWritabilityChanged() {
         let embedded = EmbeddedChannel()
         let testWriter = TestBackpressureWriter(eventLoop: embedded.eventLoop, parts: 5)