diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Manager.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Manager.swift index 3fdf93752..4c313e92b 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Manager.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Manager.swift @@ -19,24 +19,26 @@ import NIOCore import NIOHTTP1 extension HTTPConnectionPool { - final class Manager { + final class Manager: Sendable { private typealias Key = ConnectionPool.Key - private enum State { + private enum RunState: Sendable { case active case shuttingDown(promise: EventLoopPromise?, unclean: Bool) case shutDown } + private struct State: Sendable { + var runState: RunState = .active + var pools: [Key: HTTPConnectionPool] = [:] + } + private let eventLoopGroup: EventLoopGroup private let configuration: HTTPClient.Configuration private let connectionIDGenerator = Connection.ID.globalGenerator private let logger: Logger - private var state: State = .active - private var _pools: [Key: HTTPConnectionPool] = [:] - private let lock = NIOLock() - + private let state: NIOLockedValueBox = NIOLockedValueBox(State()) private let sslContextCache = SSLContextCache() init( @@ -51,10 +53,10 @@ extension HTTPConnectionPool { func executeRequest(_ request: HTTPSchedulableRequest) { let poolKey = request.poolKey - let poolResult = self.lock.withLock { () -> Result in - switch self.state { + let poolResult = self.state.withLockedValue { state -> Result in + switch state.runState { case .active: - if let pool = self._pools[poolKey] { + if let pool = state.pools[poolKey] { return .success(pool) } @@ -68,7 +70,7 @@ extension HTTPConnectionPool { idGenerator: self.connectionIDGenerator, backgroundActivityLogger: self.logger ) - self._pools[poolKey] = pool + state.pools[poolKey] = pool return .success(pool) case .shuttingDown, .shutDown: @@ -95,17 +97,17 @@ extension HTTPConnectionPool { case shutdown([Key: HTTPConnectionPool]) } - let action = self.lock.withLock { () -> ShutdownAction in - switch self.state { + let action = self.state.withLockedValue { state -> ShutdownAction in + switch state.runState { case .active: // If there aren't any pools, we can mark the pool as shut down right away. - if self._pools.isEmpty { - self.state = .shutDown + if state.pools.isEmpty { + state.runState = .shutDown return .done(promise) } else { // this promise will be succeeded once all connection pools are shutdown - self.state = .shuttingDown(promise: promise, unclean: false) - return .shutdown(self._pools) + state.runState = .shuttingDown(promise: promise, unclean: false) + return .shutdown(state.pools) } case .shuttingDown, .shutDown: @@ -135,23 +137,23 @@ extension HTTPConnectionPool.Manager: HTTPConnectionPoolDelegate { case wait } - let closeAction = self.lock.withLock { () -> CloseAction in - switch self.state { + let closeAction = self.state.withLockedValue { state -> CloseAction in + switch state.runState { case .active, .shutDown: preconditionFailure("Why are pools shutting down, if the manager did not give a signal") case .shuttingDown(let promise, let soFarUnclean): - guard self._pools.removeValue(forKey: pool.key) === pool else { + guard state.pools.removeValue(forKey: pool.key) === pool else { preconditionFailure( "Expected that the pool was created by this manager and is known for this reason." ) } - if self._pools.isEmpty { - self.state = .shutDown + if state.pools.isEmpty { + state.runState = .shutDown return .close(promise, unclean: soFarUnclean || unclean) } else { - self.state = .shuttingDown(promise: promise, unclean: soFarUnclean || unclean) + state.runState = .shuttingDown(promise: promise, unclean: soFarUnclean || unclean) return .wait } } diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index e628c6073..f516e9083 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -57,7 +57,7 @@ let globalRequestID = ManagedAtomic(0) /// } /// } /// ``` -public class HTTPClient { +public final class HTTPClient: Sendable { /// The `EventLoopGroup` in use by this ``HTTPClient``. /// /// All HTTP transactions will occur on loops owned by this group. @@ -66,11 +66,9 @@ public class HTTPClient { let poolManager: HTTPConnectionPool.Manager /// Shared thread pool used for file IO. It is lazily created on first access of ``Task/fileIOThreadPool``. - private var fileIOThreadPool: NIOThreadPool? - private let fileIOThreadPoolLock = NIOLock() + private let fileIOThreadPool: NIOLockedValueBox - private var state: State - private let stateLock = NIOLock() + private let state: NIOLockedValueBox private let canBeShutDown: Bool static let loggingDisabled = Logger(label: "AHC-do-not-log", factory: { _ in SwiftLogNoOpLogHandler() }) @@ -167,29 +165,32 @@ public class HTTPClient { configuration: self.configuration, backgroundActivityLogger: backgroundActivityLogger ) - self.state = .upAndRunning + self.state = NIOLockedValueBox(.upAndRunning) + self.fileIOThreadPool = NIOLockedValueBox(nil) } deinit { debugOnly { // We want to crash only in debug mode. - switch self.state { - case .shutDown: - break - case .shuttingDown: - preconditionFailure( - """ - This state should be totally unreachable. While the HTTPClient is shutting down a \ - reference cycle should exist, that prevents it from deinit. - """ - ) - case .upAndRunning: - preconditionFailure( - """ - Client not shut down before the deinit. Please call client.shutdown() when no \ - longer needed. Otherwise memory will leak. - """ - ) + self.state.withLockedValue { state in + switch state { + case .shutDown: + break + case .shuttingDown: + preconditionFailure( + """ + This state should be totally unreachable. While the HTTPClient is shutting down a \ + reference cycle should exist, that prevents it from deinit. + """ + ) + case .upAndRunning: + preconditionFailure( + """ + Client not shut down before the deinit. Please call client.shutdown() when no \ + longer needed. Otherwise memory will leak. + """ + ) + } } } } @@ -302,11 +303,11 @@ public class HTTPClient { return } do { - try self.stateLock.withLock { - guard case .upAndRunning = self.state else { + try self.state.withLockedValue { state in + guard case .upAndRunning = state else { throw HTTPClientError.alreadyShutdown } - self.state = .shuttingDown(requiresCleanClose: requiresCleanClose, callback: callback) + state = .shuttingDown(requiresCleanClose: requiresCleanClose, callback: callback) } } catch { callback(error) @@ -320,17 +321,16 @@ public class HTTPClient { case .failure: preconditionFailure("Shutting down the connection pool must not fail, ever.") case .success(let unclean): - let (callback, uncleanError) = self.stateLock.withLock { () -> (ShutdownCallback, Error?) in - guard case .shuttingDown(let requiresClean, callback: let callback) = self.state else { + let (callback, uncleanError) = self.state.withLockedValue { + (state: inout HTTPClient.State) -> (ShutdownCallback, Error?) in + guard case .shuttingDown(let requiresClean, callback: let callback) = state else { preconditionFailure("Why did the pool manager shut down, if it was not instructed to") } let error: Error? = (requiresClean && unclean) ? HTTPClientError.uncleanShutdown : nil + state = .shutDown return (callback, error) } - self.stateLock.withLock { - self.state = .shutDown - } queue.async { callback(uncleanError) } @@ -340,11 +340,11 @@ public class HTTPClient { @Sendable private func makeOrGetFileIOThreadPool() -> NIOThreadPool { - self.fileIOThreadPoolLock.withLock { - guard let fileIOThreadPool = self.fileIOThreadPool else { + self.fileIOThreadPool.withLockedValue { pool in + guard let pool else { return NIOThreadPool.singleton } - return fileIOThreadPool + return pool } } @@ -734,8 +734,8 @@ public class HTTPClient { ] ) - let failedTask: Task? = self.stateLock.withLock { - switch self.state { + let failedTask: Task? = self.state.withLockedValue { state in + switch state { case .upAndRunning: return nil case .shuttingDown, .shutDown: @@ -1113,9 +1113,6 @@ extension HTTPClient.Configuration: Sendable {} extension HTTPClient.EventLoopGroupProvider: Sendable {} extension HTTPClient.EventLoopPreference: Sendable {} -// HTTPClient is thread-safe because its shared mutable state is protected through a lock -extension HTTPClient: @unchecked Sendable {} - extension HTTPClient.Configuration { /// Timeout configuration. public struct Timeout: Sendable {