Skip to content
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

Sendable checking #569

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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 @@ -201,7 +201,13 @@ protocol HTTPRequestExecutor {
func cancelRequest(_ task: HTTPExecutableRequest)
}

protocol HTTPExecutableRequest: AnyObject {
#if swift(>=5.6)
typealias _HTTPExecutableRequestSendable = Sendable
#else
typealias _HTTPExecutableRequestSendable = Any
#endif

protocol HTTPExecutableRequest: AnyObject, _HTTPExecutableRequestSendable {
/// The request's logger
var logger: Logger { get }

Expand Down
5 changes: 5 additions & 0 deletions Sources/AsyncHTTPClient/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1025,3 +1025,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible {
@available(*, deprecated, message: "AsyncHTTPClient now correctly supports informational headers. For this reason `httpEndReceivedAfterHeadWith1xx` will not be thrown anymore.")
public static let httpEndReceivedAfterHeadWith1xx = HTTPClientError(code: .httpEndReceivedAfterHeadWith1xx)
}

#if swift(>=5.6)
/// HTTPClient is Sendable, since shared state is protected by the internal ``stateLock``.
extension HTTPClient: @unchecked Sendable {}
#endif
19 changes: 17 additions & 2 deletions Sources/AsyncHTTPClient/HTTPHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@

import Foundation
import Logging
import NIOConcurrencyHelpers
#if swift(>=5.6)
@preconcurrency import NIOCore
#else
import NIOCore
#endif
import NIOConcurrencyHelpers
import NIOHTTP1
import NIOSSL

Expand Down Expand Up @@ -385,6 +389,12 @@ public class ResponseAccumulator: HTTPClientResponseDelegate {
}
}

#if swift(>=5.6)
@preconcurrency public protocol _HTTPClientResponseDelegate: Sendable {}
#else
public protocol _HTTPClientResponseDelegate {}
#endif

/// `HTTPClientResponseDelegate` allows an implementation to receive notifications about request processing and to control how response parts are processed.
/// 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
/// headers before deciding whether to accept a response body, or if you want to stream your request body. Pass an instance of your conforming
Expand Down Expand Up @@ -414,7 +424,7 @@ public class ResponseAccumulator: HTTPClientResponseDelegate {
/// released together with the `HTTPTaskHandler` when channel is closed.
/// Users of the library are not required to keep a reference to the
/// object that implements this protocol, but may do so if needed.
public protocol HTTPClientResponseDelegate: AnyObject {
public protocol HTTPClientResponseDelegate: AnyObject, _HTTPClientResponseDelegate {
associatedtype Response

/// Called when the request head is sent. Will be called once.
Expand Down Expand Up @@ -635,6 +645,11 @@ extension HTTPClient {
}
}

#if swift(>=5.6)
// HTTPClient.Task is Sendable thanks to the internal lock.
extension HTTPClient.Task: @unchecked Sendable {}
#endif

internal struct TaskCancelEvent {}

// MARK: - RedirectHandler
Expand Down
5 changes: 5 additions & 0 deletions Sources/AsyncHTTPClient/RequestBag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -446,3 +446,8 @@ extension RequestBag: HTTPClientTaskDelegate {
}
}
}

#if swift(>=5.6)
// RequestBag is Sendable because everything is dispatched onto the EL.
extension RequestBag: @unchecked Sendable {}
#endif
29 changes: 17 additions & 12 deletions Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@
//===----------------------------------------------------------------------===//

@testable import AsyncHTTPClient
#if swift(>=5.6)
@preconcurrency import Logging
#else
import Logging
#endif
import NIOCore
import NIOPosix
import XCTest
Expand Down Expand Up @@ -327,13 +331,14 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
let client = makeDefaultHTTPClient()
defer { XCTAssertNoThrow(try client.syncShutdown()) }
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
var request = HTTPClientRequest(url: "http://localhost:\(bin.port)/offline")
request.method = .POST
let streamWriter = AsyncSequenceWriter<ByteBuffer>()
request.body = .stream(streamWriter, length: .unknown)

let task = Task<HTTPClientResponse, Error> { [request] in
try await client.execute(request, deadline: .now() + .seconds(2), logger: logger)
let task = Task<Void, Error> {
var request = HTTPClientRequest(url: "http://localhost:\(bin.port)/offline")
request.method = .POST
let streamWriter = AsyncSequenceWriter<ByteBuffer>()
request.body = .stream(streamWriter, length: .unknown)

_ = try await client.execute(request, deadline: .now() + .seconds(2), logger: logger)
}
task.cancel()
await XCTAssertThrowsError(try await task.value) { error in
Expand All @@ -352,10 +357,10 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
let client = makeDefaultHTTPClient()
defer { XCTAssertNoThrow(try client.syncShutdown()) }
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
let request = HTTPClientRequest(url: "https://localhost:\(bin.port)/wait")

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

let task = Task<HTTPClientResponse, Error> { [request] in
try await client.execute(request, deadline: .now(), logger: logger)
let task = Task<Void, Error> {
let request = HTTPClientRequest(url: "http://localhost:\(bin.port)/wait")
_ = try await client.execute(request, deadline: .now(), logger: logger)
}
await XCTAssertThrowsError(try await task.value) { error in
guard let error = error as? HTTPClientError else {
Expand Down
4 changes: 4 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,10 @@ extension HTTPBin where RequestHandler == HTTPBinHandler {
}
}

#if swift(>=5.6)
extension HTTPBin: @unchecked Sendable {}
#endif

enum HTTPBinError: Error {
case refusedConnection
case invalidProxyRequest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,7 @@ private class MockScheduledRequest: HTTPSchedulableRequest {
preconditionFailure("Unimplemented")
}
}

#if swift(>=5.6)
extension MockScheduledRequest: @unchecked Sendable {}
#endif
8 changes: 8 additions & 0 deletions Tests/AsyncHTTPClientTests/Mocks/MockConnectionPool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@

@testable import AsyncHTTPClient
import Logging
#if swift(>=5.6)
@preconcurrency import NIOCore
#else
import NIOCore
#endif
import NIOHTTP1
import NIOSSL

Expand Down Expand Up @@ -747,3 +751,7 @@ class MockHTTPRequest: HTTPSchedulableRequest {
preconditionFailure("Unimplemented")
}
}

#if swift(>=5.6)
extension MockHTTPRequest: @unchecked Sendable {}
#endif
10 changes: 10 additions & 0 deletions Tests/AsyncHTTPClientTests/Mocks/MockRequestExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@

@testable import AsyncHTTPClient
import NIOConcurrencyHelpers
#if swift(>=5.6)
@preconcurrency import NIOCore
#else
import NIOCore
#endif

// This is a MockRequestExecutor, that is synchronized on its EventLoop.
final class MockRequestExecutor {
Expand Down Expand Up @@ -273,3 +277,9 @@ extension MockRequestExecutor {
}
}
}

#if swift(>=5.6)
extension MockRequestExecutor: @unchecked Sendable {}
extension MockRequestExecutor.RequestParts: Sendable {}
extension MockRequestExecutor.BlockingQueue: @unchecked Sendable {}
#endif
15 changes: 12 additions & 3 deletions Tests/AsyncHTTPClientTests/RequestBagTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
@testable import AsyncHTTPClient
import Logging
import NIOCore
import NIOConcurrencyHelpers
import NIOEmbedded
import NIOHTTP1
import XCTest
Expand Down Expand Up @@ -561,16 +562,24 @@ class UploadCountingDelegate: HTTPClientResponseDelegate {
}
}

class MockTaskQueuer: HTTPRequestScheduler {
private(set) var hitCancelCount = 0
final class MockTaskQueuer: HTTPRequestScheduler {
private let hitCancelCounter = NIOAtomic<Int>.makeAtomic(value: 0)

var hitCancelCount: Int {
self.hitCancelCounter.load()
}

init() {}

func cancelRequest(_: HTTPSchedulableRequest) {
self.hitCancelCount += 1
self.hitCancelCounter.add(1)
}
}

#if swift(>=5.6)
extension MockTaskQueuer: @unchecked Sendable {}
#endif

extension RequestOptions {
static func forTests(idleReadTimeout: TimeAmount? = nil) -> Self {
RequestOptions(
Expand Down
Loading