apple / swift-nio

Event-driven network application framework for high performance protocol servers & clients, non-blocking.
https://swiftpackageindex.com/apple/swift-nio/documentation
Apache License 2.0
7.92k stars 640 forks source link

NIO can send `channelInactive` before `channelActive` reordered if user `Channel.close()`s in `connect` promise callback #2773

Open weissi opened 2 months ago

weissi commented 2 months ago

Expected behavior

Actual behavior

It's possible to get channelActive following channelInactive.

Steps to reproduce

This seems related to https://github.com/apple/swift-nio-ssl/issues/467

NIOSSLHandler observes the channel events in this order:

NIOSSLHandler sees the following channel events:

Obviously those channel events are wrong. That's a bug in NIO too.

weissi commented 2 months ago

The issues is that the connect promise gets (by design) priority over fireChannelActive. The code's here: https://github.com/apple/swift-nio/blob/abbb144f6678d0d830899b8bda5eea5924a61c0d/Sources/NIOPosix/BaseSocketChannel.swift#L131-L132

        case (.fullyRegistered, .activate):
            self.currentState = .activated
            return { promise, pipeline in
                promise?.succeed(())
                pipeline.syncOperations.fireChannelActive()
            }

So NIO (like Netty) will always fulfil the promise first and then fire the channel pipeline event. That leads to this reorder:

weissi commented 2 months ago

I think this is a rediscovery of https://github.com/apple/swift-nio/issues/196 . Please note this comment: https://github.com/apple/swift-nio/issues/196#issuecomment-374863576