-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NIOTSEventLoops don't participate in MTEventLoopGroup.currentEventLoop #86
Comments
Stating this more strongly: I don't believe this is possible, irregardless of whether it's desirable. I think a first step is: do we believe this problem has a generic solution at all? It's my understanding that in the absence of any context-propagation functionality within Dispatch it is simply not possible to implement a correct version of the "what event loop am I on" function with NIOTS. Am I missing something? |
No, you're definitely correct. But it's possible there will eventually be API in Dispatch that allows for it (miracles do happen! 😆), and even if not, it doesn't negate that I absolutely do not believe that |
There are two problems with
EventLoopGroup
as currently concretely implemented:MultiThreadedEventLoopGroup.threadSpecificEventLoop
is aThreadSpecificVariable
, which has unpredictable behavior when used with dispatch queues (which have their own mechanism for the purpose).EventLoopGroup
protocol to provide an API for expressing acurrentEventLoop
concept without making that API public.This has some unfortunate results when any alternate
EventLoopGroup
- most especiallyNIOTSEventLoopGroup
is in use:EventLoopGroup.syncShutdownGracefully()
while still on an event loop will fail to diagnose the problem via precondition as it normally would.EventLoopFuture.wait()
while executing on any event loop will incorrectly report the "current" event loop asnil
. In addition, the misuse will only be correctly detected when it takes on the future's own event loop, and even then will suffer even further fromNIOTSEventLoop.inEventLoop
's "false negative" problem.NIOPipeBootstrap.withPipes()
similarly fails to diagnose an attempt to bootstrap while on an event loop.To make matters even more confusing, it's not clear that
NIOTSEventLoop
could provide a satisfactory implementation of an API for the appropriate check in the first place, given the aforementioned problems experienced byNIOTSEventLoop.inEventLoop
. (See https://github.com/apple/swift-nio-transport-services/blob/master/Sources/NIOTransportServices/NIOTSEventLoop.swift#L84-L94 for details on that issue.)While all of these uses of
currentEventLoop
are limited to expressingprecondition()
s (which is exactly as it should be), and thus these failures are not critical, the lack of these diagnostics tends to make debugging more difficult and delays the detection of such issues until after the problem has become worse. This is not a satisfactory state of affairs, given the recommendation thatNIOTSEventLoopGroup
be preferred overMultiThreadedEventLoopGroup
whenever available - users are left with a recommended default which suffers from (admittedly minor) reduced usability.It is also possible for other alternative
EventLoopGroup
implementations to exist, such as a threading model specialized for Windows support, or just for the sheer ridiculousness of it, anEventLoop
based on Mach threads (yikes). This suggests that the problem would ideally be solved in a fully generic fashion, rather than providing some form of special-case behavior for NIOTS (if that were possible to begin with).I have not yet come up with any kind of solution which would not require one of:
currentEventLoop
as public API (which is obviously undesirable; it would only be misused in the same waydispatch_get_current_queue()
historically has been)public
but definitionally private API (as the stdlib does with underscored functions which do not appear in generated interfaces but are nonetheless usable if their names are known)I would love to discuss alternatives, if anyone has any thoughts.
The text was updated successfully, but these errors were encountered: