Skip to content

Commit 9a9ddb4

Browse files
fabianfettfabianfett
committed
Sendable checking
1 parent 42b0637 commit 9a9ddb4

11 files changed

+159
-59
lines changed

‎Sources/AsyncHTTPClient/ConnectionPool/HTTPExecutableRequest.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,13 @@ protocol HTTPRequestExecutor {
201201
func cancelRequest(_ task: HTTPExecutableRequest)
202202
}
203203

204-
protocol HTTPExecutableRequest: AnyObject {
204+
#if swift(>=5.6)
205+
typealias _HTTPExecutableRequestSendable = Sendable
206+
#else
207+
typealias _HTTPExecutableRequestSendable = Any
208+
#endif
209+
210+
protocol HTTPExecutableRequest: AnyObject, _HTTPExecutableRequestSendable {
205211
/// The request's logger
206212
var logger: Logger { get }
207213

‎Sources/AsyncHTTPClient/HTTPClient.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,3 +1025,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible {
10251025
@available(*, deprecated, message: "AsyncHTTPClient now correctly supports informational headers. For this reason `httpEndReceivedAfterHeadWith1xx` will not be thrown anymore.")
10261026
public static let httpEndReceivedAfterHeadWith1xx = HTTPClientError(code: .httpEndReceivedAfterHeadWith1xx)
10271027
}
1028+
1029+
#if swift(>=5.6)
1030+
/// HTTPClient is Sendable, since shared state is protected by the internal ``stateLock``.
1031+
extension HTTPClient: @unchecked Sendable {}
1032+
#endif

‎Sources/AsyncHTTPClient/HTTPHandler.swift

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@
1414

1515
import Foundation
1616
import Logging
17-
import NIOConcurrencyHelpers
17+
#if swift(>=5.6)
18+
@preconcurrency import NIOCore
19+
#else
1820
import NIOCore
21+
#endif
22+
import NIOConcurrencyHelpers
1923
import NIOHTTP1
2024
import NIOSSL
2125

@@ -385,6 +389,12 @@ public class ResponseAccumulator: HTTPClientResponseDelegate {
385389
}
386390
}
387391

392+
#if swift(>=5.6)
393+
@preconcurrency public protocol _HTTPClientResponseDelegate: Sendable {}
394+
#else
395+
public protocol _HTTPClientResponseDelegate {}
396+
#endif
397+
388398
/// `HTTPClientResponseDelegate` allows an implementation to receive notifications about request processing and to control how response parts are processed.
389399
/// You can implement this protocol if you need fine-grained control over an HTTP request/response, for example, if you want to inspect the response
390400
/// headers before deciding whether to accept a response body, or if you want to stream your request body. Pass an instance of your conforming
@@ -414,7 +424,7 @@ public class ResponseAccumulator: HTTPClientResponseDelegate {
414424
/// released together with the `HTTPTaskHandler` when channel is closed.
415425
/// Users of the library are not required to keep a reference to the
416426
/// object that implements this protocol, but may do so if needed.
417-
public protocol HTTPClientResponseDelegate: AnyObject {
427+
public protocol HTTPClientResponseDelegate: AnyObject, _HTTPClientResponseDelegate {
418428
associatedtype Response
419429

420430
/// Called when the request head is sent. Will be called once.
@@ -635,6 +645,11 @@ extension HTTPClient {
635645
}
636646
}
637647

648+
#if swift(>=5.6)
649+
// HTTPClient.Task is Sendable thanks to the internal lock.
650+
extension HTTPClient.Task: @unchecked Sendable {}
651+
#endif
652+
638653
internal struct TaskCancelEvent {}
639654

640655
// MARK: - RedirectHandler

‎Sources/AsyncHTTPClient/RequestBag.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,3 +446,8 @@ extension RequestBag: HTTPClientTaskDelegate {
446446
}
447447
}
448448
}
449+
450+
#if swift(>=5.6)
451+
// RequestBag is Sendable because everything is dispatched onto the EL.
452+
extension RequestBag: @unchecked Sendable {}
453+
#endif

‎Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
@testable import AsyncHTTPClient
16-
import Logging
16+
@preconcurrency import Logging
1717
import NIOCore
1818
import NIOPosix
1919
import XCTest
@@ -327,13 +327,14 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
327327
let client = makeDefaultHTTPClient()
328328
defer { XCTAssertNoThrow(try client.syncShutdown()) }
329329
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
330-
var request = HTTPClientRequest(url: "http://localhost:\(bin.port)/offline")
331-
request.method = .POST
332-
let streamWriter = AsyncSequenceWriter<ByteBuffer>()
333-
request.body = .stream(streamWriter, length: .unknown)
334330

335-
let task = Task<HTTPClientResponse, Error> { [request] in
336-
try await client.execute(request, deadline: .now() + .seconds(2), logger: logger)
331+
let task = Task<Void, Error> {
332+
var request = HTTPClientRequest(url: "http://localhost:\(bin.port)/offline")
333+
request.method = .POST
334+
let streamWriter = AsyncSequenceWriter<ByteBuffer>()
335+
request.body = .stream(streamWriter, length: .unknown)
336+
337+
_ = try await client.execute(request, deadline: .now() + .seconds(2), logger: logger)
337338
}
338339
task.cancel()
339340
await XCTAssertThrowsError(try await task.value) { error in
@@ -352,10 +353,10 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
352353
let client = makeDefaultHTTPClient()
353354
defer { XCTAssertNoThrow(try client.syncShutdown()) }
354355
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
355-
let request = HTTPClientRequest(url: "https://localhost:\(bin.port)/wait")
356356

357-
let task = Task<HTTPClientResponse, Error> { [request] in
358-
try await client.execute(request, deadline: .now() + .milliseconds(100), logger: logger)
357+
let task = Task<Void, Error> {
358+
let request = HTTPClientRequest(url: "https://localhost:\(bin.port)/wait")
359+
_ = try await client.execute(request, deadline: .now() + .milliseconds(100), logger: logger)
359360
}
360361
await XCTAssertThrowsError(try await task.value) { error in
361362
guard let error = error as? HTTPClientError else {
@@ -377,10 +378,10 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
377378
let client = makeDefaultHTTPClient()
378379
defer { XCTAssertNoThrow(try client.syncShutdown()) }
379380
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
380-
let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/wait")
381381

382-
let task = Task<HTTPClientResponse, Error> { [request] in
383-
try await client.execute(request, deadline: .now(), logger: logger)
382+
let task = Task<Void, Error> {
383+
let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/wait")
384+
_ = try await client.execute(request, deadline: .now(), logger: logger)
384385
}
385386
await XCTAssertThrowsError(try await task.value) { error in
386387
guard let error = error as? HTTPClientError else {

‎Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,10 @@ extension HTTPBin where RequestHandler == HTTPBinHandler {
591591
}
592592
}
593593

594+
#if swift(>=5.6)
595+
extension HTTPBin: @unchecked Sendable {}
596+
#endif
597+
594598
enum HTTPBinError: Error {
595599
case refusedConnection
596600
case invalidProxyRequest

‎Tests/AsyncHTTPClientTests/HTTPConnectionPool+RequestQueueTests.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,3 +137,7 @@ private class MockScheduledRequest: HTTPSchedulableRequest {
137137
preconditionFailure("Unimplemented")
138138
}
139139
}
140+
141+
#if swift(>=5.6)
142+
extension MockScheduledRequest: @unchecked Sendable {}
143+
#endif

‎Tests/AsyncHTTPClientTests/Mocks/MockConnectionPool.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@
1414

1515
@testable import AsyncHTTPClient
1616
import Logging
17+
#if swift(>=5.6)
18+
@preconcurrency import NIOCore
19+
#else
1720
import NIOCore
21+
#endif
1822
import NIOHTTP1
1923
import NIOSSL
2024

@@ -747,3 +751,7 @@ class MockHTTPRequest: HTTPSchedulableRequest {
747751
preconditionFailure("Unimplemented")
748752
}
749753
}
754+
755+
#if swift(>=5.6)
756+
extension MockHTTPRequest: @unchecked Sendable {}
757+
#endif

‎Tests/AsyncHTTPClientTests/Mocks/MockRequestExecutor.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@
1414

1515
@testable import AsyncHTTPClient
1616
import NIOConcurrencyHelpers
17+
#if swift(>=5.6)
18+
@preconcurrency import NIOCore
19+
#else
1720
import NIOCore
21+
#endif
1822

1923
// This is a MockRequestExecutor, that is synchronized on its EventLoop.
2024
final class MockRequestExecutor {
@@ -273,3 +277,9 @@ extension MockRequestExecutor {
273277
}
274278
}
275279
}
280+
281+
#if swift(>=5.6)
282+
extension MockRequestExecutor: @unchecked Sendable {}
283+
extension MockRequestExecutor.RequestParts: Sendable {}
284+
extension MockRequestExecutor.BlockingQueue: @unchecked Sendable {}
285+
#endif

‎Tests/AsyncHTTPClientTests/RequestBagTests.swift

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
@testable import AsyncHTTPClient
1616
import Logging
1717
import NIOCore
18+
import NIOConcurrencyHelpers
1819
import NIOEmbedded
1920
import NIOHTTP1
2021
import XCTest
@@ -522,16 +523,24 @@ class UploadCountingDelegate: HTTPClientResponseDelegate {
522523
}
523524
}
524525

525-
class MockTaskQueuer: HTTPRequestScheduler {
526-
private(set) var hitCancelCount = 0
526+
final class MockTaskQueuer: HTTPRequestScheduler {
527+
private let hitCancelCounter = NIOAtomic<Int>.makeAtomic(value: 0)
528+
529+
var hitCancelCount: Int {
530+
self.hitCancelCounter.load()
531+
}
527532

528533
init() {}
529534

530535
func cancelRequest(_: HTTPSchedulableRequest) {
531-
self.hitCancelCount += 1
536+
self.hitCancelCounter.add(1)
532537
}
533538
}
534539

540+
#if swift(>=5.6)
541+
extension MockTaskQueuer: @unchecked Sendable {}
542+
#endif
543+
535544
extension RequestOptions {
536545
static func forTests(idleReadTimeout: TimeAmount? = nil) -> Self {
537546
RequestOptions(

‎Tests/AsyncHTTPClientTests/TransactionTests.swift

Lines changed: 73 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
@testable import AsyncHTTPClient
1616
import Logging
1717
import NIOConcurrencyHelpers
18-
import NIOCore
18+
@preconcurrency import NIOCore
1919
import NIOEmbedded
2020
import NIOHTTP1
2121
import NIOPosix
@@ -41,23 +41,23 @@ final class TransactionTests: XCTestCase {
4141
guard let preparedRequest = maybePreparedRequest else {
4242
return XCTFail("Expected to have a request here.")
4343
}
44-
let (transaction, responseTask) = Transaction.makeWithResultTask(
44+
45+
let queuer = MockTaskQueuer()
46+
47+
await XCTAssertThrowsError(try await Transaction.awaitResponseWithTransaction(
4548
request: preparedRequest,
4649
preferredEventLoop: embeddedEventLoop
47-
)
50+
) { transaction in
51+
transaction.requestWasQueued(queuer)
4852

49-
let queuer = MockTaskQueuer()
50-
transaction.requestWasQueued(queuer)
53+
Task.detached {
54+
try await Task.sleep(nanoseconds: 5 * 1000 * 1000)
55+
transaction.cancel()
56+
}
5157

52-
Task.detached {
53-
try await Task.sleep(nanoseconds: 5 * 1000 * 1000)
54-
transaction.cancel()
55-
}
58+
XCTAssertEqual(queuer.hitCancelCount, 0)
59+
})
5660

57-
XCTAssertEqual(queuer.hitCancelCount, 0)
58-
await XCTAssertThrowsError(try await responseTask.value) {
59-
XCTAssertEqual($0 as? HTTPClientError, .cancelled)
60-
}
6161
XCTAssertEqual(queuer.hitCancelCount, 1)
6262
}
6363
#endif
@@ -78,50 +78,54 @@ final class TransactionTests: XCTestCase {
7878
guard let preparedRequest = maybePreparedRequest else {
7979
return
8080
}
81-
let (transaction, responseTask) = Transaction.makeWithResultTask(
82-
request: preparedRequest,
83-
preferredEventLoop: embeddedEventLoop
84-
)
8581

8682
let executor = MockRequestExecutor(
8783
pauseRequestBodyPartStreamAfterASingleWrite: true,
8884
eventLoop: embeddedEventLoop
8985
)
9086

91-
transaction.willExecuteRequest(executor)
92-
transaction.requestHeadSent()
93-
94-
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["foo": "bar"])
95-
XCTAssertFalse(executor.signalledDemandForResponseBody)
96-
transaction.receiveResponseHead(responseHead)
87+
let response = try await Transaction.awaitResponseWithTransaction(
88+
request: preparedRequest,
89+
preferredEventLoop: embeddedEventLoop
90+
) { transaction in
9791

98-
let response = try await responseTask.value
99-
XCTAssertEqual(response.status, responseHead.status)
100-
XCTAssertEqual(response.headers, responseHead.headers)
101-
XCTAssertEqual(response.version, responseHead.version)
92+
transaction.willExecuteRequest(executor)
93+
transaction.requestHeadSent()
10294

103-
let iterator = SharedIterator(response.body.filter { $0.readableBytes > 0 }.makeAsyncIterator())
95+
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["foo": "bar"])
96+
XCTAssertFalse(executor.signalledDemandForResponseBody)
97+
transaction.receiveResponseHead(responseHead)
10498

105-
for i in 0..<100 {
106-
XCTAssertFalse(executor.signalledDemandForResponseBody, "Demand was not signalled yet.")
99+
for i in 0..<100 {
100+
XCTAssertFalse(executor.signalledDemandForResponseBody, "Demand was not signalled yet.")
107101

108-
async let part = iterator.next()
102+
XCTAssertNoThrow(try executor.receiveResponseDemand())
103+
executor.resetResponseStreamDemandSignal()
104+
transaction.receiveResponseBodyParts([ByteBuffer(integer: i)])
105+
}
109106

107+
XCTAssertFalse(executor.signalledDemandForResponseBody, "Demand was not signalled yet.")
110108
XCTAssertNoThrow(try executor.receiveResponseDemand())
111109
executor.resetResponseStreamDemandSignal()
112-
transaction.receiveResponseBodyParts([ByteBuffer(integer: i)])
110+
transaction.succeedRequest([])
111+
}
113112

114-
let result = try await part
113+
let expectedResponse = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["foo": "bar"])
114+
115+
XCTAssertEqual(response.status, expectedResponse.status)
116+
XCTAssertEqual(response.headers, expectedResponse.headers)
117+
XCTAssertEqual(response.version, expectedResponse.version)
118+
119+
var iterator = response.body.filter { $0.readableBytes > 0 }.makeAsyncIterator()
120+
121+
for i in 0..<100 {
122+
let result = try await iterator.next()
115123
XCTAssertEqual(result, ByteBuffer(integer: i))
116124
}
117125

118-
XCTAssertFalse(executor.signalledDemandForResponseBody, "Demand was not signalled yet.")
119-
async let part = iterator.next()
120-
XCTAssertNoThrow(try executor.receiveResponseDemand())
121-
executor.resetResponseStreamDemandSignal()
122-
transaction.succeedRequest([])
123-
let result = try await part
124-
XCTAssertNil(result)
126+
let final = try await iterator.next()
127+
XCTAssertNil(final)
128+
125129
}
126130
#endif
127131
}
@@ -581,5 +585,34 @@ extension Transaction {
581585

582586
return (transaction, result)
583587
}
588+
589+
fileprivate static func awaitResponseWithTransaction(
590+
request: PreparedRequest,
591+
requestOptions: RequestOptions = .forTests(),
592+
logger: Logger = Logger(label: "test"),
593+
connectionDeadline: NIODeadline = .distantFuture,
594+
preferredEventLoop: EventLoop,
595+
_ closure: @Sendable @escaping (Transaction) async throws -> ()
596+
) async throws -> HTTPClientResponse {
597+
598+
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<HTTPClientResponse, Error>) in
599+
let transaction = Transaction(
600+
request: request,
601+
requestOptions: requestOptions,
602+
logger: logger,
603+
connectionDeadline: connectionDeadline,
604+
preferredEventLoop: preferredEventLoop,
605+
responseContinuation: continuation
606+
)
607+
608+
Task {
609+
do {
610+
try await closure(transaction)
611+
} catch {
612+
XCTFail()
613+
}
614+
}
615+
}
616+
}
584617
}
585618
#endif

0 commit comments

Comments
 (0)