make trio.BlockingTrioPortal and trio.Lock work together

[njsmith]

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 a BlockingTrioPortal – 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 do portal.run(lock.acquire), and then later portal.run_sync(lock.release), then you'll get an error, because the BlockingTrioPortal uses a different backing task for each call, so from the Lock'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. (Like threading.Lock, which is totally happy for different threads to call acquire and release.) 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 API: highlevel strategy discussion trio-asyncio#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 like with portal.async_with(lock): ..., and BlockingTrioPortal.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 a BlockingTrioPortal.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 a with portal_factory.open() as portal: ... that you have to do in the thread to get a portal handle?

• Change the trio<->thread API entirely, using something like @agronholm's async with in_trio: ... / async with in_thread: ... trick. API: highlevel strategy discussion trio-asyncio#42 has arguments for why this makes sense for trio/asyncio transitions, Should we have a way to let some other coroutine runner take temporary control of a task? #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 writing async 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 to async with in_trio, lock, in_thread: ..., but I don't know if that helps much :-).

[njsmith]

(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.)

[agronholm]

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.

[agronholm]

Doing the same from an SQLAlchemy flush/commit listener is an even better example.

[Nikratio]

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?

[njsmith]

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