Open njsmith opened 5 years ago
(It'd be great if folks who have real uses for BlockingTrioPortal
, like @nikratio and @zeed, could chime in with more details about what their code looks like.)
In my apps, I've needed the ability to run async tasks from threaded code when a function primarily run inside a worker thread needs to use an existing async service. Our Exchange integration is a good example of this. Exchangelib's API is horrendous and is highly unsafe to run in an async task since at any point it could start an outbound connection.
Doing the same from an SQLAlchemy flush/commit listener is an even better example.
My code currently uses thread/task locks only within condition objects, and I think I am fine at the moment. However, I depend on being able to acquire and release from different threads for other locks that are currently thread-only. My plan was to port those to trio, but I guess I better wait with that :-).
How about adding something like trio.DumbLock
, which is a separate, non-recursive lock that can be used when acquiring/releasing by different threads is specifically desired?
How about adding something like
trio.DumbLock
, which is a separate, non-recursive lock that can be used when acquiring/releasing by different threads is specifically desired?
Since Guido's not using his time machine currently, I borrowed it to implement this as trio.Semaphore(1, max_value=1)
.
In this SO question, @Nikratio asks about how to share a lock between Trio and a thread.
There are some advantages to using
trio.Lock
, and having the thread access it through aBlockingTrioPortal
– in particular, it means that the Trio-side code can use cancellation as normal. (It's also maybe cheaper? I haven't measured, but it seems plausible.)However, if you try this, you'll get bitten in a surprising way:
trio.Lock
tracks which task acquired it, and wants that task to release it. This is intended to help avoid mistakes and give us the option of implementing deadlock detection later (#182), but here it creates a problem: if you doportal.run(lock.acquire)
, and then laterportal.run_sync(lock.release)
, then you'll get an error, because theBlockingTrioPortal
uses a different backing task for each call, so from theLock
's perspective the task that's trying to release it is some random task unrelated to the one that acquired it.This is unfortunate, we should fix it somehow. Some options:
Make
trio.Lock
less picky about who releases it. (Likethreading.Lock
, which is totally happy for different threads to callacquire
andrelease
.) Downsides: this would mean we can never add deadlock detection or otherwise report on lock ownership when debugging; feels kind of weird and error-prone; inconsistent with recursive locks (which inherently have to track ownership). Doesn't solve the problem for other context managers that have the same issue (e.g. nurseries – see also https://github.com/python-trio/trio-asyncio/issues/42 which is about context-switching between trio and asyncio, which has essentially isomorphic concerns)Add an API
BlockingTrioPortal
to wrap a context manager, so you write something likewith portal.async_with(lock): ...
, andBlockingTrioPortal.async_with
is clever enough to allocate a single backing task and use it for calling both__aenter__
and__aexit__
.Make it so
BlockingTrioPortal
somehow maintains a single backing task across multiple operations automatically. Managing the task lifetime becomes a bit tricky here: what if multiple threads use the same portal? Do we need to add aBlockingTrioPortal.close
API to shut down the backing task(s)? I don't think there's any actual show-stoppers that would prevent killing the backing task(s) from__del__
, though of course it's pretty complicated to do. Or we could change the API to make this easier to track, e.g. by having awith portal_factory.open() as portal: ...
that you have to do in the thread to get aportal
handle?Change the trio<->thread API entirely, using something like @agronholm's
async with in_trio: ...
/async with in_thread: ...
trick. https://github.com/python-trio/trio-asyncio/issues/42 has arguments for why this makes sense for trio/asyncio transitions, #649 is the trio issue for discussing the low-level API we need in the core to make this possible. I guess whether this is a good idea depends a lot on how people are using this... do they really want to switch back and forth between trio and a thread? if so it's great. Do they just want to acquire a lock, and otherwise stay in the thread? If so then writingasync with in_trio: async with lock: async with in_thread: ...
would be pretty annoying (and there's some question about how to get back into the same thread, or if that even matters). I guess that could be shortened toasync with in_trio, lock, in_thread: ...
, but I don't know if that helps much :-).