Review changes addressed

Kitura · Oct 10, 2019 · 6a7f631 · 6a7f631
1 parent 20135a1
commit 6a7f631
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 23 deletions.
diff --git a/Package.swift b/Package.swift
@@ -28,7 +28,7 @@ let package = Package(
     ],
     dependencies: [
         // Dependencies declare other packages that this package depends on.
-        .package(url: "https://github.com/apple/swift-nio.git", from: "2.3.0"),
+        .package(url: "https://github.com/apple/swift-nio.git", .branch("master")),
         .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.0.0"),
         .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.0.0"),
         .package(url: "https://github.com/IBM-Swift/BlueSSLService.git", from: "1.0.0"),

diff --git a/Sources/KituraNet/HTTP/HTTPServer.swift b/Sources/KituraNet/HTTP/HTTPServer.swift
@@ -42,6 +42,14 @@ An HTTP server that listens for connections on a socket.
  server.stop()
 ````
 */
+
+#if os(Linux)
+    let numberOfCores = Int(linux_sched_getaffinity())
+    fileprivate let globalELG = MultiThreadedEventLoopGroup(numberOfThreads: numberOfCores > 0 ? numberOfCores : System.coreCount)
+#else
+    fileprivate let globalELG = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
+#endif
+
 public class HTTPServer: Server {
 
     public typealias ServerType = HTTPServer
@@ -122,8 +130,17 @@ public class HTTPServer: Server {
     /// Maximum number of pending connections
     private let maxPendingConnections = 100
 
-    /// The event loop group on which the HTTP handler runs
-    public let eventLoopGroup: EventLoopGroup
+    // A lazily initialized EventLoopGroup, accessed via `eventLoopGroup`
+    private var _eventLoopGroup: EventLoopGroup?
+
+    /// The EventLoopGroup used by this HTTPServer. This property may be assigned
+    /// once and once only, by calling `setEventLoopGroup(value:)` before `listen()` is called.
+    public var eventLoopGroup: EventLoopGroup {
+        if let value = self._eventLoopGroup { return value }
+        let value = globalELG
+        self._eventLoopGroup = value
+        return value
+    }
 
     var quiescingHelper: ServerQuiescingHelper?
 
@@ -137,16 +154,6 @@ public class HTTPServer: Server {
      ````
     */
     public init() {
-#if os(Linux)
-        let numberOfCores = Int(linux_sched_getaffinity())
-        self.eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: numberOfCores > 0 ? numberOfCores : System.coreCount)
-#else
-        self.eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
-#endif
-    }
-
-    public init(eventLoopGroup: EventLoopGroup) {
-        self.eventLoopGroup = eventLoopGroup
     }
 
     /**
@@ -282,6 +289,16 @@ public class HTTPServer: Server {
         try listen(.tcp(port, address))
     }
 
+    /// Sets the EventLoopGroup to be used by this HTTPServer. This may be called once
+    /// and once only, and must be called prior to `listen()`.
+    /// - Throws: If the EventLoopGroup has already been assigned.
+    public func setEventLoopGroup(_ value: EventLoopGroup) throws {
+        guard _eventLoopGroup == nil else {
+            throw HTTPServerError.eventLoopGroupAlreadyInitialized
+        }
+        _eventLoopGroup = value
+    }
+
     private func listen(_ socket: SocketType) throws {
 
         if let tlsConfig = tlsConfig {
@@ -458,14 +475,6 @@ public class HTTPServer: Server {
         return server
     }
 
-    deinit {
-        do {
-            try eventLoopGroup.syncShutdownGracefully()
-        } catch {
-            Log.error("Failed to shutdown eventLoopGroup")
-        }
-    }
-
     /**
      Stop listening for new connections.
 
@@ -678,3 +687,7 @@ enum KituraWebSocketUpgradeError: Error {
     // Unknown upgrade error
     case unknownUpgradeError
 }
+
+public enum HTTPServerError: Error {
+    case eventLoopGroupAlreadyInitialized
+}
diff --git a/Tests/KituraNetTests/RegressionTests.swift b/Tests/KituraNetTests/RegressionTests.swift
@@ -199,7 +199,12 @@ class RegressionTests: KituraNetTest {
             #else
             let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
             #endif
-            let server = HTTPServer(eventLoopGroup: eventLoopGroup)
+            let server = HTTPServer()
+            do {
+                try server.setEventLoopGroup(eventLoopGroup)
+            } catch {
+                XCTFail("Unable to initialize EventLoopGroup: \(error)")
+            }
             let serverPort: Int = 8091
             defer {
                 server.stop()
@@ -215,7 +220,13 @@ class RegressionTests: KituraNetTest {
             XCTAssertEqual(goodClient.connectedPort, serverPort, "GoodClient not connected to expected server port")
 
             // Start a server using eventLoopGroup api provided by HTPPServer()
-            let server2 = HTTPServer(eventLoopGroup: server.eventLoopGroup)
+            let server2 = HTTPServer()
+            do {
+                try server2.setEventLoopGroup(server.eventLoopGroup)
+            } catch {
+                XCTFail("Unable to initialize EventLoopGroup: \(error)")
+            }
+
             let serverPort2: Int = 8092
             defer {
                 server2.stop()
@@ -235,6 +246,29 @@ class RegressionTests: KituraNetTest {
         waitForExpectations(timeout: 10)
     }
 
+    func testFailEventLoopGroupReinitialization() {
+        do {
+        #if os(Linux)
+        let numberOfCores = Int(linux_sched_getaffinity())
+        let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: numberOfCores > 0 ? numberOfCores : System.coreCount)
+        #else
+        let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
+        #endif
+        let server = HTTPServer()
+        do {
+            try server.listen(on: 8093)
+        } catch {
+            XCTFail("Couldn't start server \(error)")
+            }
+        do {
+            try server.setEventLoopGroup(eventLoopGroup)
+        } catch {
+            let serverError = error as? HTTPServerError
+            XCTAssertEqual(serverError, HTTPServerError.eventLoopGroupAlreadyInitialized)
+            }
+        }
+    }
+
     /// A simple client which connects to a port but sends no data
     struct BadClient {
         let clientBootstrap: ClientBootstrap