Merge branch 'main' into ff-fix-benchmark-non-failure

Author: fabianfett

Sources/NIOPosix/BaseSocketChannel.swift

@@ -24,12 +24,14 @@ private struct SocketChannelLifecycleManager {
         case fresh
         case preRegistered  // register() has been run but the selector doesn't know about it yet
         case fullyRegistered  // fully registered, ie. the selector knows about it
-        case activated
+        case preActivation  // activated, but we have not fired channelActivated yet
+        case fullyActivated  // fully activated, i.e., the signal has been fired
         case closed
     }
 
     private enum Event {
-        case activate
+        case beginActivation
+        case finishActivation
         case beginRegistration
         case finishRegistration
         case close
@@ -52,9 +54,11 @@ private struct SocketChannelLifecycleManager {
         didSet {
             self.eventLoop.assertInEventLoop()
             switch (oldValue, self.currentState) {
-            case (_, .activated):
+            case (_, .preActivation):
                 self.isActiveAtomic.store(true, ordering: .relaxed)
-            case (.activated, _):
+            case (.preActivation, .fullyActivated):
+                ()  // Stay active
+            case (.preActivation, _), (.fullyActivated, _):
                 self.isActiveAtomic.store(false, ordering: .relaxed)
             default:
                 ()
@@ -99,8 +103,13 @@ private struct SocketChannelLifecycleManager {
 
     // we need to return a closure here and to not suffer from a potential allocation for that this must be inlined
     @inline(__always)
-    internal mutating func activate() -> ((EventLoopPromise<Void>?, ChannelPipeline) -> Void) {
-        self.moveState(event: .activate)
+    internal mutating func beginActivation() -> ((EventLoopPromise<Void>?, ChannelPipeline) -> Void) {
+        self.moveState(event: .beginActivation)
+    }
+
+    @inline(__always)
+    internal mutating func finishActivation() -> ((EventLoopPromise<Void>?, ChannelPipeline) -> Void) {
+        self.moveState(event: .finishActivation)
     }
 
     // MARK: private API
@@ -132,11 +141,10 @@ private struct SocketChannelLifecycleManager {
             }
 
         // origin: .fullyRegistered
-        case (.fullyRegistered, .activate):
-            self.currentState = .activated
+        case (.fullyRegistered, .beginActivation):
+            self.currentState = .preActivation
             return { promise, pipeline in
                 promise?.succeed(())
-                pipeline.syncOperations.fireChannelActive()
             }
 
         // origin: .preRegistered || .fullyRegistered
@@ -147,29 +155,61 @@ private struct SocketChannelLifecycleManager {
                 pipeline.syncOperations.fireChannelUnregistered()
             }
 
-        // origin: .activated
-        case (.activated, .close):
+        // origin: .preActivation
+        case (.preActivation, .finishActivation):
+            self.currentState = .fullyActivated
+            return { promise, pipeline in
+                assert(promise == nil)
+                pipeline.syncOperations.fireChannelActive()
+            }
+
+        case (.preActivation, .close):
+            self.currentState = .closed
+            return { promise, pipeline in
+                promise?.succeed(())
+                // Don't fire channelInactive here.
+                pipeline.syncOperations.fireChannelUnregistered()
+            }
+
+        case (.preActivation, .beginActivation) where self.supportsReconnect:
+            return { promise, pipeline in
+                promise?.succeed(())
+            }
+
+        // origin: .fullyActivated
+        case (.fullyActivated, .close):
             self.currentState = .closed
             return { promise, pipeline in
                 promise?.succeed(())
                 pipeline.syncOperations.fireChannelInactive()
                 pipeline.syncOperations.fireChannelUnregistered()
             }
 
-        // origin: .activated
-        case (.activated, .activate) where self.supportsReconnect:
+        case (.fullyActivated, .beginActivation) where self.supportsReconnect:
             return { promise, pipeline in
                 promise?.succeed(())
             }
 
+        case (.fullyActivated, .finishActivation) where self.supportsReconnect:
+            return { promise, pipeline in
+                assert(promise == nil)
+            }
+
         // bad transitions
-        case (.fresh, .activate),  // should go through .registered first
-            (.preRegistered, .activate),  // need to first be fully registered
+        case (.fresh, .beginActivation),  // should go through .registered first
+            (.fresh, .finishActivation),  // should go through .registerd first
+            (.preRegistered, .beginActivation),  // need to first be fully registered
+            (.preRegistered, .finishActivation),  // need to first be fully registered
             (.preRegistered, .beginRegistration),  // already registered
             (.fullyRegistered, .beginRegistration),  // already registered
-            (.activated, .activate),  // already activated
-            (.activated, .beginRegistration),  // already fully registered (and activated)
-            (.activated, .finishRegistration),  // already fully registered (and activated)
+            (.fullyRegistered, .finishActivation),  // need to activate first
+            (.preActivation, .beginActivation),  // already activated
+            (.preActivation, .beginRegistration),  // already fully registered (and activated)
+            (.preActivation, .finishRegistration),  // already fully registered (and activated)
+            (.fullyActivated, .beginActivation),  // need to activate first
+            (.fullyActivated, .beginRegistration),  // already fully registered
+            (.fullyActivated, .finishRegistration),  // already fully registered
+            (.fullyActivated, .finishActivation),  // already signaled
             (.fullyRegistered, .finishRegistration),  // already fully registered
             (.fresh, .finishRegistration),  // need to register lazily first
             (.closed, _):  // already closed
@@ -184,15 +224,20 @@ private struct SocketChannelLifecycleManager {
     // MARK: convenience properties
     internal var isActive: Bool {
         self.eventLoop.assertInEventLoop()
-        return self.currentState == .activated
+        switch self.currentState {
+        case .preActivation, .fullyActivated:
+            return true
+        case .fresh, .preRegistered, .fullyRegistered, .closed:
+            return false
+        }
     }
 
     internal var isPreRegistered: Bool {
         self.eventLoop.assertInEventLoop()
         switch self.currentState {
         case .fresh, .closed:
             return false
-        case .preRegistered, .fullyRegistered, .activated:
+        case .preRegistered, .fullyRegistered, .preActivation, .fullyActivated:
             return true
         }
     }
@@ -202,7 +247,7 @@ private struct SocketChannelLifecycleManager {
         switch self.currentState {
         case .fresh, .closed, .preRegistered:
             return false
-        case .fullyRegistered, .activated:
+        case .fullyRegistered, .preActivation, .fullyActivated:
             return true
         }
     }
@@ -1366,7 +1411,13 @@ class BaseSocketChannel<SocketType: BaseSocketProtocol>: SelectableChannel, Chan
                 return
             }
         }
-        self.lifecycleManager.activate()(promise, self.pipeline)
+        self.lifecycleManager.beginActivation()(promise, self.pipeline)
+        guard self.lifecycleManager.isActive else {
+            // the channel got closed in the promise succeed closure
+            return
+        }
+        // if the channel wasn't closed, continue to fully activate
+        self.lifecycleManager.finishActivation()(nil, self.pipeline)
         guard self.lifecycleManager.isOpen else {
             // in the user callout for `channelActive` the channel got closed.
             return

Tests/NIOPosixTests/ChannelNotificationTest.swift

@@ -534,4 +534,50 @@ class ChannelNotificationTest: XCTestCase {
         XCTAssertNoThrow(try serverChannel.close().wait())
         XCTAssertNoThrow(try serverChannel.closeFuture.wait())
     }
+
+    func testNoActiveInactiveNotificationsWhenDirectlyClosed() throws {
+        let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
+        defer {
+            XCTAssertNoThrow(try group.syncShutdownGracefully())
+        }
+
+        class OrderVerificationHandler: ChannelInboundHandler {
+            typealias InboundIn = ByteBuffer
+
+            init() {
+                // nop
+            }
+
+            public func channelActive(context: ChannelHandlerContext) {
+                XCTFail("There should be no events when the channel is closed directly.")
+            }
+
+            public func channelInactive(context: ChannelHandlerContext) {
+                XCTFail("There should be no events when the channel is closed directly.")
+            }
+        }
+
+        let serverChannel = try assertNoThrowWithValue(
+            ServerBootstrap(group: group)
+                .serverChannelOption(.socketOption(.so_reuseaddr), value: 1)
+                .bind(host: "127.0.0.1", port: 0).wait()
+        )
+
+        let connectFuture = ClientBootstrap(group: group)
+            .channelInitializer { channel in
+                channel.eventLoop.makeCompletedFuture {
+                    try channel.pipeline.syncOperations.addHandler(OrderVerificationHandler())
+                }
+            }
+            .connect(to: serverChannel.localAddress!)
+
+        // Close immediately in the promise callback. The channel must still become active before it becomes inactive!
+        let closeFuture = connectFuture.flatMap { channel in
+            channel.close()
+        }
+
+        XCTAssertNoThrow(try closeFuture.wait())
+
+        XCTAssertNoThrow(try serverChannel.close().wait())
+    }
 }

scripts/check-cxx-interop-compatibility.sh

@@ -22,9 +22,9 @@ fatal() { error "$@"; exit 1; }
 log "Checking for Cxx interoperability compatibility..."
 
 source_dir=$(pwd)
-working_dir=$(mktemp -d)
+working_dir=$(mktemp -d "/tmp/tmp_swift_package_XXXXXXXXXX")
 project_name=$(basename "$working_dir")
-source_file=Sources/$project_name/$(echo "$project_name" | tr . _).swift
+source_file=Sources/$project_name/$project_name.swift
 library_products=$( swift package dump-package | jq -r '.products[] | select(.type.library != null) | .name')
 package_name=$(swift package dump-package | jq -r '.name')