Skip to content

Correctly reset our state after .sendEnd #597

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ extension HTTP1ConnectionStateMachine.State {
self = .closing
newFinalAction = .close
case .sendRequestEnd(let writePromise):
self = .idle
newFinalAction = .sendRequestEnd(writePromise)
case .none:
self = .idle
Expand Down
6 changes: 5 additions & 1 deletion Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1267,7 +1267,11 @@ final class HTTP200DelayedHandler: ChannelInboundHandler {
let request = self.unwrapInboundIn(data)
switch request {
case .head:
break
// Once we have received one response, all further requests are responded to immediately.
if self.pendingBodyParts == nil {
context.writeAndFlush(self.wrapOutboundOut(.head(.init(version: .http1_1, status: .ok))), promise: nil)
context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil)
}
case .body:
if let pendingBodyParts = self.pendingBodyParts {
if pendingBodyParts > 0 {
Expand Down
1 change: 1 addition & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ extension HTTPClientTests {
("testWeCloseConnectionsWhenConnectionCloseSetByServer", testWeCloseConnectionsWhenConnectionCloseSetByServer),
("testBiDirectionalStreaming", testBiDirectionalStreaming),
("testBiDirectionalStreamingEarly200", testBiDirectionalStreamingEarly200),
("testBiDirectionalStreamingEarly200DoesntPreventUsFromSendingMoreRequests", testBiDirectionalStreamingEarly200DoesntPreventUsFromSendingMoreRequests),
("testSynchronousHandshakeErrorReporting", testSynchronousHandshakeErrorReporting),
("testFileDownloadChunked", testFileDownloadChunked),
("testCloseWhileBackpressureIsExertedIsFine", testCloseWhileBackpressureIsExertedIsFine),
Expand Down
54 changes: 54 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3021,6 +3021,60 @@ class HTTPClientTests: XCTestCase {
XCTAssertNil(try delegate.next().wait())
}

// This test is identical to the one above, except that we send another request immediately after. This is a regression
// test for https://github.com/swift-server/async-http-client/issues/595.
func testBiDirectionalStreamingEarly200DoesntPreventUsFromSendingMoreRequests() {
let httpBin = HTTPBin(.http1_1(ssl: false, compress: false)) { _ in HTTP200DelayedHandler(bodyPartsBeforeResponse: 1) }
defer { XCTAssertNoThrow(try httpBin.shutdown()) }

let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 2)
defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) }
let writeEL = eventLoopGroup.next()

let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup))
defer { XCTAssertNoThrow(try httpClient.syncShutdown()) }

let body: HTTPClient.Body = .stream { writer in
let finalPromise = writeEL.makePromise(of: Void.self)

func writeLoop(_ writer: HTTPClient.Body.StreamWriter, index: Int) {
// always invoke from the wrong el to test thread safety
writeEL.preconditionInEventLoop()

if index >= 30 {
return finalPromise.succeed(())
}

let sent = ByteBuffer(integer: index)
writer.write(.byteBuffer(sent)).whenComplete { result in
switch result {
case .success:
writeEL.execute {
writeLoop(writer, index: index + 1)
}

case .failure(let error):
finalPromise.fail(error)
}
}
}

writeEL.execute {
writeLoop(writer, index: 0)
}

return finalPromise.futureResult
}

let request = try! HTTPClient.Request(url: "http://localhost:\(httpBin.port)", body: body)
let future = httpClient.execute(request: request)
XCTAssertNoThrow(try future.wait())

// Try another request
let future2 = httpClient.execute(request: request)
XCTAssertNoThrow(try future2.wait())
}

func testSynchronousHandshakeErrorReporting() throws {
// This only affects cases where we use NIOSSL.
guard !isTestingNIOTS() else { return }
Expand Down