SSLContextCache: use DispatchQueue instead of NIOThreadPool (#368)

Motivation:

In the vast majority of cases, we'll only ever create one and only one
`NIOSSLContext`. It's therefore wasteful to keep around a whole thread
doing nothing just for that. A `DispatchQueue` is absolutely fine here.

Modification:

Run the `NIOSSLContext` creation on a `DispatchQueue` instead.

Result:

Fewer threads hanging around.

Sources/AsyncHTTPClient/ConnectionPool.swift

@@ -109,8 +109,6 @@ final class ConnectionPool {
             self.providers.values
         }
 
-        self.sslContextCache.shutdown()
-
         return EventLoopFuture.reduce(true, providers.map { $0.close() }, on: eventLoop) { $0 && $1 }
     }
 

Sources/AsyncHTTPClient/SSLContextCache.swift

@@ -12,69 +12,22 @@
 //
 //===----------------------------------------------------------------------===//
 
+import Dispatch
 import Logging
 import NIO
 import NIOConcurrencyHelpers
 import NIOSSL
 
 class SSLContextCache {
-    private var state = State.activeNoThread
     private let lock = Lock()
     private var sslContextCache = LRUCache<BestEffortHashableTLSConfiguration, NIOSSLContext>()
-    private let threadPool = NIOThreadPool(numberOfThreads: 1)
-
-    enum State {
-        case activeNoThread
-        case active
-        case shutDown
-    }
-
-    init() {}
-
-    func shutdown() {
-        self.lock.withLock { () -> Void in
-            switch self.state {
-            case .activeNoThread:
-                self.state = .shutDown
-            case .active:
-                self.state = .shutDown
-                self.threadPool.shutdownGracefully { maybeError in
-                    precondition(maybeError == nil, "\(maybeError!)")
-                }
-            case .shutDown:
-                preconditionFailure("SSLContextCache shut down twice")
-            }
-        }
-    }
-
-    deinit {
-        assert(self.state == .shutDown)
-    }
+    private let offloadQueue = DispatchQueue(label: "io.github.swift-server.AsyncHTTPClient.SSLContextCache")
 }
 
 extension SSLContextCache {
-    private struct SSLContextCacheShutdownError: Error {}
-
     func sslContext(tlsConfiguration: TLSConfiguration,
                     eventLoop: EventLoop,
                     logger: Logger) -> EventLoopFuture<NIOSSLContext> {
-        let earlyExitError: Error? = self.lock.withLock { () -> Error? in
-            switch self.state {
-            case .activeNoThread:
-                self.state = .active
-                self.threadPool.start()
-                return nil
-            case .active:
-                return nil
-            case .shutDown:
-                return SSLContextCacheShutdownError()
-            }
-        }
-
-        if let error = earlyExitError {
-            return eventLoop.makeFailedFuture(error)
-        }
-
         let eqTLSConfiguration = BestEffortHashableTLSConfiguration(wrapping: tlsConfiguration)
         let sslContext = self.lock.withLock {
             self.sslContextCache.find(key: eqTLSConfiguration)
@@ -88,7 +41,7 @@ extension SSLContextCache {
 
         logger.debug("creating new SSL context",
                      metadata: ["ahc-tls-config": "\(tlsConfiguration)"])
-        let newSSLContext = self.threadPool.runIfActive(eventLoop: eventLoop) {
+        let newSSLContext = self.offloadQueue.asyncWithFuture(eventLoop: eventLoop) {
             try NIOSSLContext(configuration: tlsConfiguration)
         }
 

Tests/AsyncHTTPClientTests/SSLContextCacheTests+XCTest.swift

@@ -25,10 +25,9 @@ import XCTest
 extension SSLContextCacheTests {
     static var allTests: [(String, (SSLContextCacheTests) -> () throws -> Void)] {
         return [
-            ("testJustStartingAndStoppingAContextCacheWorks", testJustStartingAndStoppingAContextCacheWorks),
             ("testRequestingSSLContextWorks", testRequestingSSLContextWorks),
-            ("testRequestingSSLContextAfterShutdownThrows", testRequestingSSLContextAfterShutdownThrows),
             ("testCacheWorks", testCacheWorks),
+            ("testCacheDoesNotReturnWrongEntry", testCacheDoesNotReturnWrongEntry),
         ]
     }
 }

Tests/AsyncHTTPClientTests/SSLContextCacheTests.swift

@@ -18,45 +18,47 @@ import NIOSSL
 import XCTest
 
 final class SSLContextCacheTests: XCTestCase {
-    func testJustStartingAndStoppingAContextCacheWorks() {
-        SSLContextCache().shutdown()
-    }
-
     func testRequestingSSLContextWorks() {
         let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
         let eventLoop = group.next()
         let cache = SSLContextCache()
         defer {
             XCTAssertNoThrow(try group.syncShutdownGracefully())
-            cache.shutdown()
         }
 
         XCTAssertNoThrow(try cache.sslContext(tlsConfiguration: .forClient(),
                                               eventLoop: eventLoop,
                                               logger: HTTPClient.loggingDisabled).wait())
     }
 
-    func testRequestingSSLContextAfterShutdownThrows() {
+    func testCacheWorks() {
         let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
         let eventLoop = group.next()
         let cache = SSLContextCache()
         defer {
             XCTAssertNoThrow(try group.syncShutdownGracefully())
         }
 
-        cache.shutdown()
-        XCTAssertThrowsError(try cache.sslContext(tlsConfiguration: .forClient(),
-                                                  eventLoop: eventLoop,
-                                                  logger: HTTPClient.loggingDisabled).wait())
+        var firstContext: NIOSSLContext?
+        var secondContext: NIOSSLContext?
+
+        XCTAssertNoThrow(firstContext = try cache.sslContext(tlsConfiguration: .forClient(),
+                                                             eventLoop: eventLoop,
+                                                             logger: HTTPClient.loggingDisabled).wait())
+        XCTAssertNoThrow(secondContext = try cache.sslContext(tlsConfiguration: .forClient(),
+                                                              eventLoop: eventLoop,
+                                                              logger: HTTPClient.loggingDisabled).wait())
+        XCTAssertNotNil(firstContext)
+        XCTAssertNotNil(secondContext)
+        XCTAssert(firstContext === secondContext)
     }
 
-    func testCacheWorks() {
+    func testCacheDoesNotReturnWrongEntry() {
         let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
         let eventLoop = group.next()
         let cache = SSLContextCache()
         defer {
             XCTAssertNoThrow(try group.syncShutdownGracefully())
-            cache.shutdown()
         }
 
         var firstContext: NIOSSLContext?
@@ -65,11 +67,13 @@ final class SSLContextCacheTests: XCTestCase {
         XCTAssertNoThrow(firstContext = try cache.sslContext(tlsConfiguration: .forClient(),
                                                              eventLoop: eventLoop,
                                                              logger: HTTPClient.loggingDisabled).wait())
-        XCTAssertNoThrow(secondContext = try cache.sslContext(tlsConfiguration: .forClient(),
+
+        // Second one has a _different_ TLSConfiguration.
+        XCTAssertNoThrow(secondContext = try cache.sslContext(tlsConfiguration: .forClient(certificateVerification: .none),
                                                               eventLoop: eventLoop,
                                                               logger: HTTPClient.loggingDisabled).wait())
         XCTAssertNotNil(firstContext)
         XCTAssertNotNil(secondContext)
-        XCTAssert(firstContext === secondContext)
+        XCTAssert(firstContext !== secondContext)
     }
 }