trio
trio copied to clipboard
We should do a better job of preserving contextvars across mode switches
When run_sync_in_worker_thread
or BlockingTrioPortal
switch between the trio thread and a worker thread, they should preserve the contextvars.Context
.
Using literally the same Context
turns out to be quite tricky though, because Context.run
enforces that only one thread can be inside a given Context
object at any given time. For run_sync_in_worker_thread
this is doable in theory but tricky (you have to make sure that the worker thread waits for the parent task to have suspended itself before it calls Context.run
), and for BlockingTrioPortal
is basically not possible. Also, if PEP 568 is merged, then we'll actually have a context stack that we would want to be preserving, and that makes things complicated too.
Instead, we should probably say that these mode switching functions copy the context (which is super cheap). This means the call would see all the same contextvars that the parent does, except that mutations in the child would not be propagated back to the parent. Which is maybe a tiny wart conceptually but seems fine in practice.
So:
-
run_sync_in_worker_thread
should docopy_context
in the trio thread and thenctx.run(...)
in the worker thread- And it should unset the
sniffio
magic contextvar in the child
- And it should unset the
-
BlockingTrioPortal
should docopy_context
in the blocking thread, and then use the context in Trio- But Trio doesn't currently have a way to say "use this context for this task", so I guess we'd need to add that. A
context=
argument tospawn_system_task
would work, and we might as well add it tostart_soon
while we're at it.- I guess these will also want to set up the
sniffio
magic contextvar. Maybe they should always copy the context that's passed in, and then mutate the copy?
- I guess these will also want to set up the
- But Trio doesn't currently have a way to say "use this context for this task", so I guess we'd need to add that. A
pytest-trio would also use a start_soon(..., context=...)
kwarg to share contexts between fixtures: https://github.com/python-trio/pytest-trio/issues/59 – but it wants start_soon
to not make a copy of the context, so maybe that's better and we should either (a) document that the Context
will be mutated to set the sniffio
state, or (b) document that people who use context=
need to set the sniffio state correctly themselves.
I guess that if PEP 568 happens and we start storing cancel state inside the context (to make it safe to use cancel scopes inside generators, see #264), then we'll need to fix up the cancel scope state whenever we get a context=
passed in from outside. So probably better to be consistent and either go ahead and mutate the sniffio
state too, or else make the copy and mutate it (and pytest-trio would just have to find a way to live with copies, which I think it can do). ...and actually I guess if we start storing cancel state inside the context then pytest-trio will have to stop using its current hack.
Also, if we had PEP 568, then I guess we wouldn't need to mutate the context to handle sniffio state; instead we could stash the sniffio state in some lower-level context on the context stack, instead of inside the individual task contexts.
Hmm. I'm pretty indecisive about copying the context versus not. Not copying is more direct (the thing you pass as context=
is the context), and versatile (if people want copies they can do that themselves). These are both good for a weird edge-case thing like this. But OTOH copying is a kind of safer, less error-prone kind of thing to do, which is also good for a weird edge-case thing like this.
Personally I'm all for copying the context. It's a different environment, after all, and I won't expect anything (besides the return value / exception) to implicitly pass back to the caller.
I added a possible implementation in this PR https://github.com/python-trio/trio/pull/2160
I looked at the release notes and realized these names have been deprecated/replaced by other things:
run_sync_in_worker_thread
-> to_thread.run_sync
BlockingTrioPortal
-> from_thread.run_sync
and from_thread.run
I would also imagine that the "sniffio
magic contextvar" refers to sniffio.current_async_library_cvar
that seems to tell the current async backend.
I'm also adding this comment here in case that PR doesn't work for some reason. I would think the information about the new names might be useful still. 🤓