Merge pull request #68 from swhitty/fix-double-close

Sockets must only be closed a single time
swhitty · Sep 18, 2023 · 4f4db93 · 4f4db93
2 parents 743be61 + c3ac2aa
commit 4f4db93
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 5 deletions.
diff --git a/FlyingFox/Sources/HTTPServer.swift b/FlyingFox/Sources/HTTPServer.swift
@@ -69,15 +69,21 @@ public final actor HTTPServer {
     }
 
     public func start() async throws {
-        let socket = try await preparePoolAndSocket()
+        guard state == nil else {
+            logger?.logCritical("server error: already started")
+            throw SocketError.unsupportedAddress
+        }
+        defer { state = nil }
         do {
+            let socket = try await preparePoolAndSocket()
             let task = Task { try await start(on: socket, pool: pool) }
             state = (socket: socket, task: task)
-            defer { state = nil }
             try await task.getValue(cancelling: .whenParentIsCancelled)
         } catch {
             logger?.logCritical("server error: \(error.localizedDescription)")
-            try? socket.close()
+            if let state = self.state {
+                try? state.socket.close()
+            }
             throw error
         }
     }
@@ -101,6 +107,7 @@ public final actor HTTPServer {
     /// - Parameter timeout: Seconds to allow for connections to close before server task is cancelled.
     public func stop(timeout: TimeInterval = 0) async {
         guard let (socket, task) = state else { return }
+        state = nil
         try? socket.close()
         for connection in connections {
             await connection.complete()

diff --git a/FlyingFox/Tests/HTTPServerTests.swift b/FlyingFox/Tests/HTTPServerTests.swift
@@ -47,16 +47,56 @@ final class HTTPServerTests: XCTestCase {
         return try await server.waitForListeningPort()
     }
 
-    func startServer(_ server: HTTPServer) async throws {
+    @discardableResult
+    func startServer(_ server: HTTPServer) async throws -> Task<Void, Error> {
         self.stopServer = server
-        Task { try await server.start() }
+        let task = Task { try await server.start() }
         try await server.waitUntilListening()
+        return task
     }
 
     override func tearDown() async throws {
         await stopServer?.stop(timeout: 0)
     }
 
+    func testThrowsError_WhenAlreadyStarted() async throws {
+        let server = HTTPServer.make()
+        try await startServer(server)
+
+        await AsyncAssertThrowsError(try await server.start(), of: SocketError.self) {
+            XCTAssertEqual($0, .unsupportedAddress)
+        }
+    }
+
+    func testRestarts_AfterStopped() async throws {
+        let server = HTTPServer.make()
+        try await startServer(server)
+        await server.stop()
+
+        await AsyncAssertNoThrow(
+            try await startServer(server)
+        )
+    }
+
+    func testTaskCanBeCancelled() async throws {
+        let server = HTTPServer.make()
+        let task = try await startServer(server)
+
+        task.cancel()
+
+        await AsyncAssertThrowsError(try await task.value)
+    }
+
+    func testTaskCanBeCancelled_AfterServerIsStopped() async throws {
+        let server = HTTPServer.make()
+        let task = try await startServer(server)
+
+        await server.stop()
+        task.cancel()
+
+        await AsyncAssertThrowsError(try await task.value)
+    }
+
     func testRequests_AreMatchedToHandlers_ViaRoute() async throws {
         let server = HTTPServer.make()
 
@@ -411,3 +451,4 @@ extension Task where Success == Never, Failure == Never {
         try await sleep(nanoseconds: UInt64(1_000_000_000 * seconds))
     }
 }
+