Make Request Sendable (#3093)

* Add some missing Sendable annotation * Start making request sendable * Making more things Sendable * More request stuff is Sendable * More Requset Sendability * Request is now Sendable * Fix Sendable warning in HTTPCookies * Yeah didn't need this * Remove Routes warning * Remove warnings from routes * Make most of ServeCommand Sendable * More warnings are gone * Suppress warnings in tests * Bypass the lock where we can * Make most of Route Sendable * Make Route Sendable * Fix some warnings on Linux * Add dev container files * Fix some test warnings on Linux * Just use the ConsoleKit version * Point to new ConsoleKit API as that's what we're using * Introduce SendableRoute * Migrate over RouteBuilder stuff * Migrate more stuff over * Migrate websockets * Fix some deprecation warnings * Try to find a way around the issue * Remove devcontainer files * Explicit use of self in HTTPCookies * Less granular locking on ServeCommand * Back out SendableRoute until we work out how to solve it
vapor · Nov 6, 2023 · 3744d42 · 3744d42
1 parent 55fc3de
commit 3744d42
Show file tree

Hide file tree

Showing 23 changed files with 358 additions and 145 deletions.
diff --git a/.gitignore b/.gitignore
@@ -7,6 +7,8 @@ Package.resolved
 .swiftpm
 Tests/LinuxMain.swift
 .vscode
+.bash_history
+.cache/
 
 # API Docs Generation
 generate-package-api-docs.swift

diff --git a/Package.swift b/Package.swift
@@ -22,7 +22,7 @@ let package = Package(
         .package(url: "https://github.com/vapor/async-kit.git", from: "1.15.0"),
 
         // 💻 APIs for creating interactive CLI tools.
-        .package(url: "https://github.com/vapor/console-kit.git", from: "4.9.0"),
+        .package(url: "https://github.com/vapor/console-kit.git", from: "4.10.0"),
 
         // 🔑 Hashing (SHA2, HMAC), encryption (AES), public-key (RSA), and random data generation.
         .package(url: "https://github.com/apple/swift-crypto.git", "1.0.0" ..< "4.0.0"),

diff --git a/Package@swift-5.9.swift b/Package@swift-5.9.swift
@@ -22,7 +22,7 @@ let package = Package(
         .package(url: "https://github.com/vapor/async-kit.git", from: "1.15.0"),
 
         // 💻 APIs for creating interactive CLI tools.
-        .package(url: "https://github.com/vapor/console-kit.git", from: "4.9.0"),
+        .package(url: "https://github.com/vapor/console-kit.git", from: "4.10.0"),
 
         // 🔑 Hashing (SHA2, HMAC), encryption (AES), public-key (RSA), and random data generation.
         .package(url: "https://github.com/apple/swift-crypto.git", "1.0.0" ..< "4.0.0"),

diff --git a/Sources/Development/routes.swift b/Sources/Development/routes.swift
@@ -247,19 +247,16 @@ public func routes(_ app: Application) throws {
         return String(buffer: body)
     }
 
-    func asyncRouteTester(_ req: Request) async throws -> String {
+    asyncRoutes.get("client2") { req -> String in
         let response = try await req.client.get("https://www.google.com")
         guard let body = response.body else {
             throw Abort(.internalServerError)
         }
         return String(buffer: body)
     }
-    asyncRoutes.get("client2", use: asyncRouteTester)
 
-    asyncRoutes.get("content", use: asyncContentTester)
-
-    func asyncContentTester(_ req: Request) async throws -> Creds {
-        return Creds(email: "name", password: "password")
+    asyncRoutes.get("content") { req in
+        Creds(email: "name", password: "password")
     }
 
     asyncRoutes.get("content2") { req async throws -> Creds in

diff --git a/Sources/Vapor/Commands/ServeCommand.swift b/Sources/Vapor/Commands/ServeCommand.swift
@@ -1,5 +1,4 @@
-import Foundation
-@preconcurrency import Dispatch
+@preconcurrency import Foundation
 import ConsoleKit
 import NIOConcurrencyHelpers
 
@@ -38,16 +37,20 @@ public final class ServeCommand: Command, Sendable {
     public var help: String {
         return "Begins serving the app over HTTP."
     }
+
+    struct SendableBox: Sendable {
+        var didShutdown: Bool
+        var running: Application.Running?
+        var signalSources: [DispatchSourceSignal]
+        var server: Server?
+    }
 
-    private var signalSources: [DispatchSourceSignal]
-    private var didShutdown: Bool
-    private var server: Server?
-    private var running: Application.Running?
+    private let box: NIOLockedValueBox<SendableBox>
 
     /// Create a new `ServeCommand`.
     init() {
-        self.signalSources = []
-        self.didShutdown = false
+        let box = SendableBox(didShutdown: false, signalSources: [])
+        self.box = .init(box)
     }
 
     /// See `Command`.
@@ -71,12 +74,13 @@ public final class ServeCommand: Command, Sendable {
         default: throw Error.incompatibleFlags
         }
 
-        self.server = context.application.server
+        var box = self.box.withLockedValue { $0 }
+        box.server = context.application.server
 
         // allow the server to be stopped or waited for
         let promise = context.application.eventLoopGroup.next().makePromise(of: Void.self)
         context.application.running = .start(using: promise)
-        self.running = context.application.running
+        box.running = context.application.running
 
         // setup signal sources for shutdown
         let signalQueue = DispatchQueue(label: "codes.vapor.server.shutdown")
@@ -87,24 +91,27 @@ public final class ServeCommand: Command, Sendable {
                 promise.succeed(())
             }
             source.resume()
-            self.signalSources.append(source)
+            box.signalSources.append(source)
             signal(code, SIG_IGN)
         }
         makeSignalSource(SIGTERM)
         makeSignalSource(SIGINT)
+        self.box.withLockedValue { $0 = box }
     }
 
     func shutdown() {
-        self.didShutdown = true
-        self.running?.stop()
-        if let server = self.server {
+        var box = self.box.withLockedValue { $0 }
+        box.didShutdown = true
+        box.running?.stop()
+        if let server = box.server {
             server.shutdown()
         }
-        self.signalSources.forEach { $0.cancel() } // clear refs
-        self.signalSources = []
+        box.signalSources.forEach { $0.cancel() } // clear refs
+        box.signalSources = []
+        self.box.withLockedValue { $0 = box }
     }
 
     deinit {
-        assert(self.didShutdown, "ServeCommand did not shutdown before deinit")
+        assert(self.box.withLockedValue({ $0.didShutdown }), "ServeCommand did not shutdown before deinit")
     }
 }
diff --git a/Sources/Vapor/Concurrency/Request+Concurrency.swift b/Sources/Vapor/Concurrency/Request+Concurrency.swift
@@ -98,7 +98,7 @@ extension Request.Body: AsyncSequence {
     ///
     /// Example: app.on(.POST, "/upload", body: .stream) { ... }
     private func checkBodyStorage() {
-        switch request.bodyStorage {
+        switch request.bodyStorage.withLockedValue({ $0 }) {
         case .stream(_):
             break
         case .collected(_):

diff --git a/Sources/Vapor/Concurrency/RoutesBuilder+Concurrency.swift b/Sources/Vapor/Concurrency/RoutesBuilder+Concurrency.swift
@@ -9,110 +9,110 @@ extension RoutesBuilder {
         _ path: PathComponent...,
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.GET, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func get<Response>(
         _ path: [PathComponent],
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.GET, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func post<Response>(
         _ path: PathComponent...,
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.POST, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func post<Response>(
         _ path: [PathComponent],
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.POST, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func patch<Response>(
         _ path: PathComponent...,
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.PATCH, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func patch<Response>(
         _ path: [PathComponent],
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.PATCH, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func put<Response>(
         _ path: PathComponent...,
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.PUT, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func put<Response>(
         _ path: [PathComponent],
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.PUT, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func delete<Response>(
         _ path: PathComponent...,
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.DELETE, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func delete<Response>(
         _ path: [PathComponent],
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(.DELETE, path, use: closure)
     }
-
+    
     @discardableResult
     @preconcurrency
     public func on<Response>(
@@ -121,13 +121,13 @@ extension RoutesBuilder {
         body: HTTPBodyStreamStrategy = .collect,
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         return self.on(method, path, body: body, use: { request in
             return try await closure(request)
         })
     }
-
+    
     @discardableResult
     @preconcurrency
     public func on<Response>(
@@ -136,7 +136,7 @@ extension RoutesBuilder {
         body: HTTPBodyStreamStrategy = .collect,
         use closure: @Sendable @escaping (Request) async throws -> Response
     ) -> Route
-        where Response: AsyncResponseEncodable
+    where Response: AsyncResponseEncodable
     {
         let responder = AsyncBasicResponder { request in
             if case .collect(let max) = body, request.body.data == nil {

diff --git a/Sources/Vapor/Concurrency/WebSocket+Concurrency.swift b/Sources/Vapor/Concurrency/WebSocket+Concurrency.swift
@@ -31,6 +31,7 @@ extension Request {
     }
 }
 
+// MARK: Deprecated
 extension RoutesBuilder {
 
     /// Adds a route for opening a web socket connection