python-trio / trio

Trio – a friendly Python library for async concurrency and I/O
https://trio.readthedocs.io
Other
6.1k stars 332 forks source link

Better usability for threads #810

Open njsmith opened 5 years ago

njsmith commented 5 years ago

I was ok with run_sync_in_worker_thread having a slightly-awkward name because threads are going to make your life awkward so, you know, it's fair warning. But when you're stuck using threads to hack around missing trio libraries, it can feel like a bit of extra punishment on top, which is not so nice.

Maybe we should rename it to run_sync_in_thread?

njsmith commented 5 years ago

Well, that seems popular.

Related idea: the whole "portal" design seems a bit more awkward than I was anticipating. I was imagining that threads were a mostly-for-experts kind of thing, and "foreign" threads would be as common as trio-spawned threads, but I feel like maybe it's not turning out that way.

Alternative idea (inspired by curio and anyio): provide a standard way to re-enter trio from a trio thread. Basically stash the token in thread-local storage, and then use it.

One possible API:

trio.run_from_thread(async_fn, *args, *, token=None)
trio.run_sync_from_thread(fn, *args, *, token=None)
trio.sync_cm_from_thread(cm, *, token=None)
trio.async_cm_from_thread(cm, *, token=None)

trio.run_sync_in_thread(fn, *args, *, ...)
trio.sync_cm_in_thread(cm, *, ...)

So the general idea is that from_thread is for going thread->trio, and in_thread is for going trio->thread. (It's taken for granted that one end is trio, because of course trio is the center of the universe. Also it's in the name of the functions.)

This pattern might be used for other projects too, like trio-asyncio – run_in_aio, run_from_aio. trio-asyncio has really struggled with making these names easy to understand – @smurfix, do you think this would do any better?

Alternatively, we could group these into a namespace, like:

trio.from_thread.run(...)
trio.from_thread.run_sync(...)
trio.from_thread.sync_cm(...)
trio.from_thread.async_cm(...)

trio.in_thread.run_sync(...)
trio.in_thread.sync_cm(...)

This would group related functions together for tab-completion, and avoid "polluting" the main trio namespace. OTOH it looks a bit weird, and flat is better than nested...

Another alternative, or complement: we could do async with trio.in_thread: .../async with trio.from_thread – see https://github.com/python-trio/trio-asyncio/issues/42 for a discussion of why this is nice – in particular it automatically works for context managers.

See also: #680 for context managers across threads, #606 for propagating cancellation across trio->thread->trio transitions, and #648 for preserving contextvars across thread switches

smurfix commented 5 years ago
trio.from_thread.async_cm(...)
trio.in_thread.run_sync(...)

The opposite of "from" is "to", so to_thread.* would work for me. in_thread is confusing because I read that as "I'm supposed to call these in a thread".

njsmith commented 5 years ago

Another possible approach:

trio.run_sync_in_thread(...)

trio.threadsafe.check_cancelled()
trio.threadsafe.run_in_trio(...)
trio.threadsafe.run_sync_in_trio(...)
trio.threadsafe.async_cm(...)

The idea is that in a thread, you're only allowed to use threadsafe functions (which makes the safety rules very easy to document!), and otherwise it uses the same naming conventions as regular trio.

njsmith commented 5 years ago

Another reason to reconsider the BlockingTrioPortal API: it's not currently compatible with having a global deadlock detector (#1085).

njsmith commented 5 years ago

I guess a downside to trio.threadsafe is that it suggests that they're safe to call anywhere, but we probably don't want allow them to be called from inside the trio thread itself.

I think these are unambiguous and fairly terse:

# These are the ones that are safe to use from a thread
trio.from_thread.run(...)
trio.from_thread.run_sync(...)
trio.from_thread.cm(...)
trio.from_thread.async_cm(...)
trio.from_thread.check_cancel()

# These are what you use from Trio
trio.to_thread.run_sync(...)
trio.to_thread.cm(...)

Alternative bikeshed color no. 1:

trio.to_trio.run(...)
trio.to_trio.run_sync(...)
trio.to_trio.cm(...)
trio.to_trio.async_cm(...)
trio.to_trio.check_cancel()

trio.to_thread.run_sync(...)
trio.to_thread.cm(...)

Alternative bikeshed color no. 2:

trio.trioify.run(...)
trio.trioify.run_sync(...)
trio.trioify.cm(...)
trio.trioify.async_cm(...)
trio.trioify.check_cancel()

trio.threadify.run_sync(...)
trio.threadify.cm(...)

Alternative bikeshed no. 3:

trio.as_trio.run(...)
trio.as_trio.run_sync(...)
trio.as_trio.cm(...)
trio.as_trio.async_cm(...)
trio.as_trio.check_cancel()

trio.as_thread.run_sync(...)
trio.as_thread.cm(...)

Alternative bikeshed no. I lost count:

# These are the ones that are safe to use from a thread
trio.from_thread.run(...)
trio.from_thread.run_sync(...)
trio.from_thread.cm(...)
trio.from_thread.async_cm(...)
trio.from_thread.check_cancel()

# These are what you use from Trio
trio.use_thread.run_sync(...)
trio.use_thread.cm(...)

Hmm. I think I prefer the "from thread" style over the "to trio" style, because if you're starting in asyncio or something then you need a different way to get "to trio". These are specifically: to trio from a thread that's running synchronous, blocking code. Which "from thread" doesn't exactly say, but if you had from_asyncio and from_thread next to each other then I think it would be pretty clear what each one meant, while if you had to_trio_from_asyncio and to_trio next to each other then it would just be confusing.

OTOH, trio.to_thread is OK because like, obviously you are starting from Trio. You don't use trio.to_thread to get from asyncio to a worker thread.

There's still the use_thread vs to_thread. Doesn't seem like a huge difference, and I guess to is shorter and more parallel, so maybe we'll just go with to. I'm pleased at how trio.run_sync_in_thread and trio.to_thread.run_sync have the same number of characters, and mostly the same characters at that.

The context manager terminology is a bit confusing:

Maybe it's better to explicitly use sync_cm & async_cm? I doubt there's any perfect answer here, because we're not going to rename trio.run to trio.run_async, and the larger Python community is never going to rename context manager → sync context manager, async context manager → context manager, so some inconsistency is unavoidable.

For the unusual case where someone wants to get into Trio from a thread that wasn't started by Trio: one option would be to have an object that implements the same stuff as from_thread. But another interesting possibility would be to lean into the thread-local idea – Trio threads get some kind of "entry handle" stashed in a thread-local by default, or, if you have an "entry handle" object, you could temporarily stash it in the thread-local by doing something like:

with entry_handle.register_thread():
    trio.from_thread.run(...)

Possibly this should automatically close the handle at the end?

I guess there's an argument for using a ContextVar instead of a thread-local. Imagine two trio threads talking to each other. Within a single trio thread, two different branches of the task tree might each want their own entry handles, maybe? Seems obscure but using a ContextVar is probably just as easy and fast as a thread-local, so why not.

njsmith commented 5 years ago

Maybe #1099 was a mistake though... if we're going to settle on trio.to_thread.run_sync as the final name, then asking everyone to make a detour through run_sync_in_thread is silly.

pquentin commented 5 years ago

(Note that it's cheap to change #1099 before the next release.)

epellis commented 5 years ago

I am interested in working on this so that #1085 can be unblocked. From what I'm gathering from this conversation, what we want to do is change how Trio calls into threads by deprecating BlockingTrioPortal and renaming it into trio.from_thread.* in a similar manner as #1098 did for trio calling into threads, by keeping track of how many trio tokens are live and might call back into the Trio thread at a given time. Is this the right line of thinking or do we want to do something different here?

njsmith commented 5 years ago

@epellis OK good question :-)

So there are a few different issues with BlockingTrioPortal:

So trio.from_thread.run and friends are designed to address all these issues:

So that's the big idea. But, we don't have to solve all of these problems at once – in particular, it's probably simplest to start by implementing a basic version of trio.from_thread.run and friends and deprecating BlockingTrioPortal, and then after that's working we can think about how to integrate the new run_sync_soon API in #1098, and about how to make them support cancellation properly. And in fact, to deprecate BlockingTrioPortal, we don't even need the context manager helpers like trio.from_thread.sync_cm, since BlockingTrioPortal doesn't have those either.

So I think the next step is to implement trio.from_thread.run, trio.from_thread.run_sync, and make it so you can run this program:

import trio

def thread_fn():
    start = trio.from_thread.run_sync(trio.current_time)
    print("In Trio-land, the time is now:", start)
    trio.from_thread.run(trio.sleep, 1)
    end = trio.from_thread.run_sync(trio.current_time)
    print("And now it's:", end)

async def main():
    await trio.run_sync_in_thread(thread_fn)

trio.run(main)
njsmith commented 5 years ago

1122 converted the old BlockingTrioPortal operations into functions trio.from_thread, and made it so they automagically work if called from inside a thread spawned by Trio. #1156 completes the renaming of trio.run_sync_in_worker_thread to trio.to_thread.run_sync. So I think we're done with refactoring the old stuff.

Not done yet:

These are interesting/tricky because they need to use the same task/thread to call both __enter__ and __exit__.

That means they might overlap with #606, which would also involve using the same task for multiple entries to trio (https://github.com/python-trio/trio/issues/606#issuecomment-515667078)

MauiJerry commented 5 years ago

latest install 0.12.0, gives message...

TrioDeprecationWarning: trio.run_sync_in_worker_thread is deprecated since Trio 0.12.0; use trio.to_thread.to_thread_run_sync instead (https://github.com/python-trio/trio/issues/810)

however, in to_thread.py it says...

from ._threads import to_thread_run_sync as run_sync

thus the correct code should be

      stuff = await trio.to_thread.run_sync(input, cancellable=True)

(from the pynng example pair1_async.py

njsmith commented 5 years ago

@MauiJerry Nice catch! That should be fixed by #1177.

njsmith commented 4 years ago

Speaking of using the same task when entering Trio repeatedly from the same thread: we should probably also do the reverse. If you go thread→trio→back to thread, we should re-use the original thread, instead of creating a new one.

This is important for two reasons.

  1. Because of OS limitations, concurrent threads are a limited resource. If we can accomplish the same thing with fewer threads, then we should.

  2. Because concurrent threads are a limited resource, we use capacity limiters to avoid creating too many threads, and in this situation that creates a deadlock hazard (!). When going thread→trio→thread, we might not be able to enter the inner thread until some existing thread exits, but the outer thread can't exit until we enter the inner thread... so if you have enough threads going back-and-forth like this, then there's a risk that trio.to_thread could lock up entirely for the whole process. But, if we're in a situation where trio.to_thread can reuse the outer thread, then we know that we've already budgeted for that thread, so we don't need to decrement the capacity limiter a second time. (I.e., when re-entering an existing thread then trio.to_thread should ignore (!) the capacity limiter argument.)

There's a bit of a subtlety to doing this optimally. The key property is that trio.to_thread can reuse a thread iff the thread is blocking in a trio.from_thread call, and that trio.from_thread call cannot return until after the trio.to_thread call returns.

I think that means: If the trio.to_thread and trio.from_thread are in the same task, then it's definitely safe. And, it's also safe if the trio.to_thread is in a different task, but that task is running inside a nursery, and that nursery is itself inside the trio.from_thread, then it's also safe. But note, this isn't quite the same as "the task is a descendent of the trio.from_thread task", because that task could have created a nursery before the initial call to trio.to_thread.

Also, we have to be careful that if we go thread→trio→spawn two tasks→both tasks go back to a thread, then only one of them can re-use the pre-existing thread at a time.

I think we can manage all this if our threading code maintains a global table of which threads are currently blocked in trio.from_thread and available to use, and for each one tracks (a) which trio Task object is hosting the trio.from_thread code, plus (b) which nurseries were already open in that Task before it started running the trio.from_thread code. Then when we enter trio.to_thread, we can check whether the current Task appears in that table, or walk up the task/nursery tree to see if the current Task is transitively inside a nursery that was newly created in one of the host Tasks after it started running the trio.from_thread code.

Phew.

arthur-tacca commented 3 years ago

Not sure if this belongs here, but it fits the issue title. I would find it quite useful if there were an additional API:

trio.from_thread.cancel(cancel_scope)

Critically, note that there's no need for the Trio token - it would work from any thread and automatically figure out the Trio thread (or threads? but presumably it's an error to use the same cancellation scope in multiple threads at once). Or, if the CancelScope isn't being used from any threads, it would just be set to cancelled in case it gets used later.

Before I tell you my motivation, let me fully acknowledge that it's quite a small convenience, so if this requires a lot of machinary behind the scenes then it's surely not worth it. But I thought I should at least mention this use case, as it could be quite common (certainly it is for me).

Imagine that you want to use Trio but have some application that has its own top-level code that you don't want to convert to Trio (shocking I know), so you spawn a thread, run Trio in it for a bit (passing back data e.g. with a queue.Queue) then cancel from the originating thread. You have to do something like this:

# variables stored somewhere accessible to both threads
cancel_scope = None
trio_token = None
# ... in trio thread ...
cancel_scope = trio.CancelScope()
trio_token = trio.lowlevel.current_trio_token()
with cancel_scope:
    # ... do some async stuff! ...
# ... in main thread, when I'm done ...
trio.from_thread.run_sync(cancel_scope.cancel, trio_token=trio_token)
# probably followed by my_thread.join()

That's a fair amount of boilerplate, whereas starting Trio in a thread to do a bit of work is otherwise suprisingly little code. It's also a bit racy (what if the variables are still None when I try to use them?); that's fixable but adds even more boilerplate.

But actually it's quite close to being a lot shorter: I could've instantiated the CancelScope right at the start before spawning the Trio thread. It's only the need to pass the TrioToken to run_sync that stops a really compact solution:

# variables stored somewhere accessible to both threads
cancel_scope = trio.CancelScope()
# ... in trio thread ...
with cancel_scope:
    # ... do some async stuff! ...
# ... in main thread, when I'm done ...
trio.from_thread.run_sync(cancel_scope.cancel, trio_token=???)

If you could replace the last line with trio.from_thread.cancel(cancel_scope) then it would all be a lot neater. I could imagine similar demand for other Trio primatives (particularly parking lot), but for me the cancellation scope is the only one that matters.

arthur-tacca commented 3 years ago

I just discovered that my first snippet is racy in two ways. I already identified that we might try to stop the Trio thread before it has started (so trio_token is still None). But we also might try to stop it after it's already complete (in which case trio_token is invalid (you get a trio.RunFinishedError error).