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.97k stars 652 forks source link

EmbeddedEventLoop/Channel violates Sendable: Store the current thread in `init()` and then assert that we're on that thread #2949

Closed weissi closed 23 hours ago

weissi commented 1 day ago

EmbeddedChannel & EmbeddedEventLoop currently violate Sendable. They should store the current thread in init and then implement inEventLoop with precondition(self.myThread == Thread.current); return true.

Otherwise, EmbeddedChannel/EventLoop are just trivially violating the rules.

Lukasa commented 1 day ago

Yes, there's no dispute about this. They have always historically broken the rules, and that remains true today.

weissi commented 1 day ago

Yes, there's no dispute about this. They have always historically broken the rules, and that remains true today.

Right, I have no idea why we didn't put that into NIO 1.0... The real selectors (kqueue/epoll/uring) do actually have myThread and are asserting it -- despite the fact that they really don't need it, that would be an insane bug if that ever failed :P

weissi commented 1 day ago

@Lukasa Do you think this needs to be staged with a NIO_STRICT thing? This might break random test suites

Lukasa commented 1 day ago

Yeah, I think it does. Environment variable is an option, another option is to just have a static flag on EmbeddedEventLoop that you can use to flip the behaviour..

weissi commented 1 day ago

Yeah, I think it does. Environment variable is an option, another option is to just have a static flag on EmbeddedEventLoop that you can use to flip the behaviour..

cool, done