Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c79e779

Browse files
fabianfettfabianfett
committedMar 17, 2022
Sendable checking
1 parent 42b0637 commit c79e779

11 files changed

+165
-57
lines changed
 

‎Sources/AsyncHTTPClient/ConnectionPool/HTTPExecutableRequest.swift

+7-1
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

+5
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

+17-2
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

+5
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

+17-12
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
@testable import AsyncHTTPClient
16+
#if swift(>=5.6)
17+
@preconcurrency import Logging
18+
#else
1619
import Logging
20+
#endif
1721
import NIOCore
1822
import NIOPosix
1923
import XCTest
@@ -327,13 +331,14 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
327331
let client = makeDefaultHTTPClient()
328332
defer { XCTAssertNoThrow(try client.syncShutdown()) }
329333
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)
334334

335-
let task = Task<HTTPClientResponse, Error> { [request] in
336-
try await client.execute(request, deadline: .now() + .seconds(2), logger: logger)
335+
let task = Task<Void, Error> {
336+
var request = HTTPClientRequest(url: "http://localhost:\(bin.port)/offline")
337+
request.method = .POST
338+
let streamWriter = AsyncSequenceWriter<ByteBuffer>()
339+
request.body = .stream(streamWriter, length: .unknown)
340+
341+
_ = try await client.execute(request, deadline: .now() + .seconds(2), logger: logger)
337342
}
338343
task.cancel()
339344
await XCTAssertThrowsError(try await task.value) { error in
@@ -352,10 +357,10 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
352357
let client = makeDefaultHTTPClient()
353358
defer { XCTAssertNoThrow(try client.syncShutdown()) }
354359
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
355-
let request = HTTPClientRequest(url: "https://localhost:\(bin.port)/wait")
356360

357-
let task = Task<HTTPClientResponse, Error> { [request] in
358-
try await client.execute(request, deadline: .now() + .milliseconds(100), logger: logger)
361+
let task = Task<Void, Error> {
362+
let request = HTTPClientRequest(url: "https://localhost:\(bin.port)/wait")
363+
_ = try await client.execute(request, deadline: .now() + .milliseconds(100), logger: logger)
359364
}
360365
await XCTAssertThrowsError(try await task.value) { error in
361366
guard let error = error as? HTTPClientError else {
@@ -377,10 +382,10 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
377382
let client = makeDefaultHTTPClient()
378383
defer { XCTAssertNoThrow(try client.syncShutdown()) }
379384
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
380-
let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/wait")
381385

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

‎Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

+4
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

+4
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

+8
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

+10
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

+12-3
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

+76-39
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515
@testable import AsyncHTTPClient
1616
import Logging
1717
import NIOConcurrencyHelpers
18+
#if swift(>=5.6)
19+
@preconcurrency import NIOCore
20+
#else
1821
import NIOCore
22+
#endif
1923
import NIOEmbedded
2024
import NIOHTTP1
2125
import NIOPosix
@@ -41,23 +45,23 @@ final class TransactionTests: XCTestCase {
4145
guard let preparedRequest = maybePreparedRequest else {
4246
return XCTFail("Expected to have a request here.")
4347
}
44-
let (transaction, responseTask) = Transaction.makeWithResultTask(
48+
49+
let queuer = MockTaskQueuer()
50+
51+
await XCTAssertThrowsError(try await Transaction.awaitResponseWithTransaction(
4552
request: preparedRequest,
4653
preferredEventLoop: embeddedEventLoop
47-
)
54+
) { transaction in
55+
transaction.requestWasQueued(queuer)
4856

49-
let queuer = MockTaskQueuer()
50-
transaction.requestWasQueued(queuer)
57+
Task.detached {
58+
try await Task.sleep(nanoseconds: 5 * 1000 * 1000)
59+
transaction.cancel()
60+
}
5161

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

57-
XCTAssertEqual(queuer.hitCancelCount, 0)
58-
await XCTAssertThrowsError(try await responseTask.value) {
59-
XCTAssertEqual($0 as? HTTPClientError, .cancelled)
60-
}
6165
XCTAssertEqual(queuer.hitCancelCount, 1)
6266
}
6367
#endif
@@ -78,50 +82,54 @@ final class TransactionTests: XCTestCase {
7882
guard let preparedRequest = maybePreparedRequest else {
7983
return
8084
}
81-
let (transaction, responseTask) = Transaction.makeWithResultTask(
82-
request: preparedRequest,
83-
preferredEventLoop: embeddedEventLoop
84-
)
8585

8686
let executor = MockRequestExecutor(
8787
pauseRequestBodyPartStreamAfterASingleWrite: true,
8888
eventLoop: embeddedEventLoop
8989
)
9090

91-
transaction.willExecuteRequest(executor)
92-
transaction.requestHeadSent()
91+
let response = try await Transaction.awaitResponseWithTransaction(
92+
request: preparedRequest,
93+
preferredEventLoop: embeddedEventLoop
94+
) { transaction in
9395

94-
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["foo": "bar"])
95-
XCTAssertFalse(executor.signalledDemandForResponseBody)
96-
transaction.receiveResponseHead(responseHead)
96+
transaction.willExecuteRequest(executor)
97+
transaction.requestHeadSent()
9798

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)
99+
let responseHead = HTTPResponseHead(version: .http1_1, status: .ok, headers: ["foo": "bar"])
100+
XCTAssertFalse(executor.signalledDemandForResponseBody)
101+
transaction.receiveResponseHead(responseHead)
102102

103-
let iterator = SharedIterator(response.body.filter { $0.readableBytes > 0 }.makeAsyncIterator())
103+
for i in 0..<100 {
104+
XCTAssertFalse(executor.signalledDemandForResponseBody, "Demand was not signalled yet.")
104105

105-
for i in 0..<100 {
106-
XCTAssertFalse(executor.signalledDemandForResponseBody, "Demand was not signalled yet.")
107-
108-
async let part = iterator.next()
106+
XCTAssertNoThrow(try executor.receiveResponseDemand())
107+
executor.resetResponseStreamDemandSignal()
108+
transaction.receiveResponseBodyParts([ByteBuffer(integer: i)])
109+
}
109110

111+
XCTAssertFalse(executor.signalledDemandForResponseBody, "Demand was not signalled yet.")
110112
XCTAssertNoThrow(try executor.receiveResponseDemand())
111113
executor.resetResponseStreamDemandSignal()
112-
transaction.receiveResponseBodyParts([ByteBuffer(integer: i)])
114+
transaction.succeedRequest([])
115+
}
113116

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

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)
130+
let final = try await iterator.next()
131+
XCTAssertNil(final)
132+
125133
}
126134
#endif
127135
}
@@ -581,5 +589,34 @@ extension Transaction {
581589

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

0 commit comments

Comments
 (0)
Please sign in to comment.