ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern #2142

weissi · 2022-06-01T09:16:42Z

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.

The text was updated successfully, but these errors were encountered:

Lukasa · 2022-06-01T09:23:52Z

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?

weissi · 2022-06-01T09:30:32Z

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?

Lukasa · 2022-06-01T09:43:33Z

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.

FranzBusch · 2022-06-01T09:59:02Z

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.

0xTim · 2022-06-01T10:01:33Z

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?

weissi · 2022-06-01T10:27:18Z

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 EventLoopGroup and an HTTPClient (and probably even more stuff in the future) to everywhere.
RFC: design suggestion: Make this a "3-tier library" swift-server/async-http-client#392

Lukasa · 2022-06-01T12:08:39Z

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.

weissi · 2022-06-01T12:11:57Z

@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.

Lukasa · 2022-06-01T12:13:51Z

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.

weissi · 2022-06-01T12:19:44Z

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?

EventLoopGroupProvider has 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.

Lukasa · 2022-06-01T13:21:12Z

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.

weissi · 2022-06-01T14:20:57Z

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.

Mordil · 2022-11-20T21:11:24Z

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.

### Motivation: SwiftNIO allows and encourages to precisely manage all operating system resources like file descriptors & threads. That is a very important property of the system but in many places -- especially since Swift Concurrency arrived -- many simpler SwiftNIO programs only require a single, globally shared EventLoopGroup. Often even with just one thread. Long story short: Many, probably most users would happily trade precise control over the threads for not having to pass around `EventLoopGroup`s. Today, many of those users resort to creating (and often leaking) threads because it's simpler. Adding a `.globalSingle` static var which _lazily_ provides singleton `EventLoopGroup`s and `NIOThreadPool`s is IMHO a much better answer. Finally, this aligns SwiftNIO a little more with Apple's SDKs which have a lot of global singletons that hold onto system resources (`Dispatch`'s thread pool, `URLSession.shared`, ...). At least in `Dispatch`'s case the Apple SDKs actually make it impossible to manage the resources, there can only ever be one global pool of threads. That's fine for app development but wouldn't be good enough for certain server use cases, therefore I propose to add `NIOSingleton`s as an _option_ to the user. Confident programmers (especially in libraries) are still free and encouraged to manage all the resources deterministically and explicitly. Companion PRs: - apple/swift-nio-transport-services#180 - swift-server/async-http-client#697 ### Modifications: - Add the `NIOSingletons` type - Add `MultiThreadedEventLoopGroup.singleton` - Add `NIOThreadPool.singleton` ### Result: - Easier use of NIO that requires fewer parameters for users who don't require full control. - Helps with #2142 - Fixes #2472 - Partially addresses #2473

weissi changed the title ~~ecosystem issue: EventLoopGroupProvider~~ ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern Jun 1, 2022

Lukasa mentioned this issue Nov 21, 2022

Getting message: BUG in SwiftNIO (please report), unleakable promise leaked #2201

Open

This was referenced Jul 13, 2023

NIOSingletons: Use NIO in easy mode #2471

Merged

NIOSingletonsTransportServices: Use NIOTS in easy mode apple/swift-nio-transport-services#180

Merged

weissi added a commit to weissi/swift-nio that referenced this issue Jul 21, 2023

deprecate NIOEventLoopGroupProvider (apple#2142)

e104959

weissi linked a pull request Jul 21, 2023 that will close this issue

deprecate NIOEventLoopGroupProvider (#2142) #2480

Draft

weissi added a commit to weissi/swift-nio that referenced this issue Jul 21, 2023

deprecate NIOEventLoopGroupProvider (apple#2142)

f44fbbe

1proprogrammerchant mentioned this issue Jul 28, 2023

NIOSingletons: Use NIO in easy mode (#2471) 1proprogrammerchant/swift-nio#3

Merged

weissi mentioned this issue Aug 9, 2023

Document the suggested way to get an EventLoopGroup #2498

Open

weissi mentioned this issue Apr 4, 2024

#700 & #701: HTTPClient.shared a globally shared singleton swift-server/async-http-client#705

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern #2142

ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern #2142

weissi commented Jun 1, 2022

Lukasa commented Jun 1, 2022

weissi commented Jun 1, 2022 •

edited

Lukasa commented Jun 1, 2022

FranzBusch commented Jun 1, 2022

0xTim commented Jun 1, 2022

weissi commented Jun 1, 2022 •

edited

Lukasa commented Jun 1, 2022

weissi commented Jun 1, 2022 •

edited

Lukasa commented Jun 1, 2022

weissi commented Jun 1, 2022

Lukasa commented Jun 1, 2022

weissi commented Jun 1, 2022 •

edited

Mordil commented Nov 20, 2022

ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern #2142

ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern #2142

Comments

weissi commented Jun 1, 2022

Lukasa commented Jun 1, 2022

weissi commented Jun 1, 2022 • edited

Lukasa commented Jun 1, 2022

FranzBusch commented Jun 1, 2022

0xTim commented Jun 1, 2022

weissi commented Jun 1, 2022 • edited

Lukasa commented Jun 1, 2022

weissi commented Jun 1, 2022 • edited

Lukasa commented Jun 1, 2022

weissi commented Jun 1, 2022

Lukasa commented Jun 1, 2022

weissi commented Jun 1, 2022 • edited

Mordil commented Nov 20, 2022

weissi commented Jun 1, 2022 •

edited

weissi commented Jun 1, 2022 •

edited

weissi commented Jun 1, 2022 •

edited

weissi commented Jun 1, 2022 •

edited