swift-nio
swift-nio copied to clipboard
apple
ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern
The NIO ecosystem suffers from one composability issue: EventLoopGroupProvider and similar constructs. The issue is that taking EventLoopGroupProvider (or similar) means that a library can either own or not own an EventLoopGroup.
That doesn't sound too bad but unfortunately, it's very bad for the shutdown case.
A library (like say AsyncHTTPClient) would obviously want to have a method called thing.shutdown() -> EventLoopFuture<Void> to match the other functions. Unfortunately, that can't be done because the library may own the EventLoopGroup, therefore shutdown() has to may have to shut it down which then however means that there is no more EventLoop left to run the future on.
The current hack is to instead provide a shutdown(group: DispatchGroup, _ completion: @escaping (Error?) -> Void) method. But that's both ugly (doesn't fit the rest) and wasteful (needs to spin up a dispatch thread for no reason).
Unfortunately, this issue won't just magically be fixed by the universal adoption of Swift Concurrency either. Yes, it's possible to provide a func shutdown() async throws -> Void but that'll then bounce from a Concurrency executor to the EventLoop to a DispatchQueue thread back to some Concurrency executor. That's still not great.
Also, there's another issue if EventLoop(Group)s frequently go away: What do we do with people who caught a reference to an EL(G) and then (after the shutdown) execute stuff on it? Currently we print this weird warning that we'll crash in future releases. That's cool if EL(G)s are mostly global/created in main but it really doesn't work if they're frequently created/destroyed.
Also there's a question w.r.t. Custom Executors (once they actually arrive in Swift): Will they support this use case?
func myFunc() async throws -> Void {
let httpClient = HTTPClient(eventLoopGroup: .createNew)
// on some Concurrency executor
do {
let result = try await httpClient.get(...)
// on a NIO EventLoop Custom Executor
print(result)
// we should be sure that this is actually legal to shut down the executor we're currently
try await httpClient.shutdown() // this is also ugly because no async in defer
} catch {
try await httpClient.shutdown() // this is also ugly because no async in defer
}
}
My recommendations are:
- deprecate
EventLoopGroupProvider - deprecate the pattern that an instance of a library (like
HTTPClient) creates its own EL(G) threads
If we did deprecate EventLoopGroupProvider and the whole pattern, what should users do who don't already have an EL(G)? This will also become even more prevalent in the future where every user-facing API will be Swift Concurrency?
The only solution I can come up with is for either SwiftNIO itself or every library lazily creating an internal ELG that will never be shut down. In other words: A global EL(G) singleton that gets created on first use, much like Dispatch or Swift Concurrency work. From a resource perspective and also alignment with Swift Concurrency it might actually make sense if SwiftNIO owned that singleton EL(G). That'd mean the new pattern to replace EventLoopGroupProvider would be .shared(elg) or .global.
The alternative to SwiftNIO owning one global ELG would be ofc for every library owning its own global ELG that could maybe just have 1 thread. I.e. AsyncHTTPClient would internally have a singleton ELG with just one thread.
What do you all think about this? None of this is fully baked but I'm really quite sure that the EventLoopGroupProvider (and similar) pattern is really bad.
Hmm, I agree that the pattern is ugly, but do we think it rises to "really bad" in a way that justifies having a singleton event loop group?
Hmm, I agree that the pattern is ugly, but do we think it rises to "really bad" in a way that justifies having a singleton event loop group?
I hate global singletons with a passion. But:
- Apple's SDK loves singletons (URLSession's connection pools/cookie storage/..., Dispatch's thread pool, Swift Concurrency's cooperative thread pool, ...). That makes NIO-style APIs more complicated even if you don't need the flexibility. This will become more of a problem once Swift Concurrency APIs are everywhere because then the NIO APIs aren't user-facing anymore to users of say AsyncHTTPClient but we'd still need an ELG. Users of AHC don't even know what an ELG is I think
- What else can we do to fix the current situation? I think the create-or-share pattern is just busted so the only two options that are left (that I can think of) are 1) force to share (bad for usability for new users & esp. for the Swift Concurrency future) 2) global or share (with more discussion needed how global global is (one ELG in SwiftNIO vs. one small ELG in each library).
So I think the prior art from the Apple SDKs and the lack of real alternatives lead to me believe that it's the least of all the evils. Any better ideas?
I mean, the simplest idea is the one that we have been falling back to all along: creating the ELG is the app's business, not the business of any library. This is coherent (apps have lifecycles, libraries don't) and customizable. It's also easier to manage and test.
I somewhat agree that the EventLoopGroupProvider is problematic; however, I am leaning towards what @Lukasa is suggesting here that libs should never create/own ELGs and this is up to the application to provide.
I mean, the simplest idea is the one that we have been falling back to all along: creating the ELG is the app's business, not the business of any library. This is coherent (apps have lifecycles, libraries don't) and customizable. It's also easier to manage and test.
To chime in here - in the world where Swift Concurrency is everywhere and libraries like Vapor, AHC etc don't expose or have ELGs in their public interfaces, forcing it to be passed around is confusing for an end user, which I think is the point @weissi was making. Is this something that goes solved eventually with custom executors?
I mean, the simplest idea is the one that we have been falling back to all along: creating the ELG is the app's business, not the business of any library.
Ideologically, I agree with you. Practically, I don't. People just don't want to carry random amount of stuff through their whole application. That's also why we created the ELGProvider pattern in AHC, it was just not acceptable to force everybody to import NIO and create an ELG. Especially given that the correct type of ELG also somewhat depends on your OS (NIO on Sockets vs. NIOTS).
This is coherent (apps have lifecycles, libraries don't) and customizable. It's also easier to manage and test.
Well, certain objects (like HTTPClient in AHC) will need lifecycle anyway to get rid of some resources but in general I get your point and again, ideologically agreed.
To chime in here - in the world where Swift Concurrency is everywhere and libraries like Vapor, AHC etc don't expose or have ELGs in their public interfaces, forcing it to be passed around is confusing for an end user, which I think is the point @weissi was making.
Exactly. I think Vapor did this really neatly by just (by default) owning the ELG in the App. And because Vapor's a framework that works just fine. You create your Vapor.Application in main and then you also own an ELG.
AsyncHTTPClient and other libraries (that aren't the application framework) this is much harder because they shouldn't [resources]/can't [lifecycle] all own their ELGs for every HTTPClient.
In this issue, I'd like to discuss the use case for libraries, i.e. things that might be used as an implementation detail in some other library/application where it'd be awkward to require the initialisation of something in main that then needs to be passed around.
Right now, this is actually even harder because of issues like
- Universal sharing is really hard/cumbersome because then users would need to carry around an
EventLoopGroupand anHTTPClient(and probably even more stuff in the future) to everywhere. - https://github.com/swift-server/async-http-client/issues/392
An implicit event loop group is still going to require someone to choose what the ELG is, or we'll commit a layering violation. NIOCore does not have an event loop group available to it, so it can't make a default choice for the user. We could let each module attempt to register whatever ELG it provides as the default, but only one of these can win, so if you've imported NIOPosix and NIOTransportServices and NIOEmbedded then you're in real trouble, because each of those provides its own ELG (and NIOEmbedded provides two). If we're expecting this to work well for users then this conflict must have a deterministic resolution.
Additionally, Swift provides no mechanism for running module constructors so in practice it is entirely possible that none of these modules have executed any code at the point when someone tries to use a default implicit ELG, at which point the only thing we can do is crash (again, NIOCore can't fix this for you). We could work around that by having libraries perform fallbacks to initialise the default if it's uninitialised at the point of use, but at this point we've largely just reinvented the existing problem.
We could work around that by having each module define its own implicit global event loop group (and then the library just has to choose which of those implicit loops it wants to use), but at that stage we end up with the result that there are several implicit global event loop groups. I'm honestly not sure this solution is better than the problem we currently have.
@Lukasa or we could have a new repo, let's call it swift-nio-family which has a NIOGlobalExecutor or so module or whatever which depends on everything and provides an ELG that it thinks is best.
Then AHC and others can depend on just swift-nio-family and unless the users passes an ELG to share, they'll all just share the NIOGlobalExecutor one.
WDYT? That'd also keep the singletons out of the core NIO repos.
Yup, we can do that, but then we get two new problems:
- Users can't avoid depending on NIOPosix if they don't want it, e.g. for binary size reasons
- We've established a "must be at least this tall" bar for ELGs to actually be useful, making it much harder for 3rd party ELGs to actually get used.
- Correct
- This is already the case today. Libraries like AHC/gRPC choose NIO on Sockets or NIOTS but they don't just choose others.
I'm very open to better, more open and composable solutions but I do think we need to address the real issue here which can probably be summed up as
There is nothing good that a library can do today.
Why?
-
EventLoopGroupProviderhas very bad API (forcing lifecycle managements, weird shutdown signature) consequences and just isn't good - users don't want to pass ELGs around unless they understand why. So if a library doesn't provide a
.createNew/.global/something easy then users feel that library is overly complicated.
I think my position is that we have no good options. It is not at all clear to me that replacing an ugly cleanup pattern with implicit global state solves more problems than it creates. We'll certainly have different problems, but that's not the same as fewer.
We're fundamentally trying to thread a needle that no-one else has to thread: we want to make it possible to use libraries without any configuration, but enable extensive configuration when users want it, with a distributed set of possible implementations where users may want to avoid having any particular one of them present. That's just gonna be hard. We're probably going to have to decide what problem we think is most important and accept that we're going to rule out a few others.
My position is that we have to do something because it forces this bad API choice on everybody who consumes AHC or other libraries using this pattern.
And fact is that with Swift Concurrency being the future, most of the code is running on a global singleton pool anyway, already.
Where are we in this discussion?
I haven't seen much reference to this discussion in the SSWG meeting notes since June, and the last discussion was had in June.
