python-trio / trio-asyncio

a re-implementation of the asyncio mainloop on top of Trio
Other
187 stars 37 forks source link

trio-asyncio should use Trio guest mode if asyncio starts first #133

Open oremanj opened 6 months ago

oremanj commented 6 months ago

Retire SyncTrioEventLoop entirely. trio-asyncio should interoperate with whatever regular asyncio event loop is running, using Trio's guest mode feature (which did not exist when trio-asyncio was first written) to manage a Trio run. This will fix #132 and make trio-asyncio a somewhat 'better citizen' to import.

jakkdl commented 5 months ago

How much work is implementing this? Feels like a big improvement and bound to improve a bunch of minor things.

oremanj commented 5 months ago

137 makes this less urgent; it provides an alternate fix for #132 that is easier to implement because it's more similar to the current approach. Using guest mode would still be better than using greenlets for most purposes, but it will run into challenges when trying to have multiple trio-asyncio event loops in the same program, and probably some other tricky edge cases esides - this is not a common use case, but the asyncio unit tests themselves do exercise it.

smurfix commented 5 months ago

IIRC guest mode hooks into the current non-Trio event loop transparently, i.e. you can run multiple Trio guests in parallel, no problem whatsoever.

Surprise, Trio's low-level reference documentation already has code that does a Trio guest run on top of asyncio. So that part is almost absurdly easy (20 lines of boilerplate-ish code). The problem is that we want to do an asyncio(-ish) guest run on top of Trio … ideally without forcing people to change their event loop startup code.

oremanj commented 5 months ago

You can't run multiple Trio guest runs in the same thread because they will all try to use the same thread-local trio._core._run.GLOBAL_RUN_CONTEXT. You can maybe do some trickery to save and restore the Trio context, similar to what I did in #137, but it's not trivial.

smurfix commented 5 months ago

Ugh. I missed that.

On the other hand, this thread-local predates contextvars AFAIK, so maybe it's time to simply replace it.

oremanj commented 5 months ago

I would be very hesitant to locate the Trio run using contextvars; they propagate in weird ways (including across thread boundaries, which we don't want for this purpose) and it would introduce a reference cycle to every Trio task (task -> context -> task). For that matter, I think locating the trio-asyncio loop using contextvars was probably a mistake also.

smurfix commented 5 months ago

Well, given that there can be more than one trio-asyncio loop at a time, offhand I don't see a better way.