Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

to/from_thread and contextvars #363

Closed
Kyuuhachi opened this issue Aug 22, 2021 · 8 comments · Fixed by #390
Closed

to/from_thread and contextvars #363

Kyuuhachi opened this issue Aug 22, 2021 · 8 comments · Fixed by #390
Labels
bug Something isn't working

Comments

@Kyuuhachi
Copy link

The behavior of how contextvars are propagated through to/from_thread differs depending on which backend you use. With asyncio, they inherit the context of the calling thread; in trio they inherit the context of the system task group.

import contextvars
import anyio

var = contextvars.ContextVar("var", default=None)

async def get():
	return var.get()

def anyio_from_thread():
	var.set("b")
	return anyio.from_thread.run(get)

async def anyio_to_from_thread():
	var.set("a")
	return await anyio.to_thread.run_sync(anyio_from_thread)

print("asyncio", anyio.run(anyio_to_from_thread, backend="asyncio"))
print("trio   ", anyio.run(anyio_to_from_thread, backend="trio"))

On anyio 3.3.0, this prints:

asyncio b
trio    None

This is easy (but inconvenient) to work around for run_sync by adding a copy_context().run, but I think any similar workaround for run would require special-casing for trio.

Also, asyncio.to_thread copies the context whereas anyio.to_thread.run_sync does not:

import contextvars
import anyio
import asyncio

var = contextvars.ContextVar("var", default=None)

async def asyncio_to_thread():
	var.set("b")
	return await asyncio.to_thread(var.get)

async def anyio_to_thread():
	var.set("b")
	return await anyio.to_thread.run_sync(var.get)

print("asyncio", asyncio.run(asyncio_to_thread()))
print("anyio  ", anyio.run(anyio_to_thread, backend="asyncio"))
asyncio c
anyio   None

This is not as important since they are different functions, but it's still annoying. Same workaround as above works here, of course.

@agronholm agronholm added the bug Something isn't working label Aug 22, 2021
@agronholm
Copy link
Owner

Which do you feel would be the correct way of propagating context?

@Kyuuhachi
Copy link
Author

I think the most intuitive would be that both to_thread and from_thread inherit the context of the caller. I haven't investigated how contextvars are used in the wider ecosystem, but that would allow libraries like Eliot to be used without lots of extra cruft.

@agronholm
Copy link
Owner

I think the most intuitive would be that both to_thread and from_thread inherit the context of the caller.

Having thought about this some more, I agree. It seems though that I cannot force trio to do this when calling back to async-land from a worker thread. Asyncio, on the other hand, behaves this way naturally.

@smurfix
Copy link
Collaborator

smurfix commented Nov 14, 2021

Then maybe we should add that feature to Trio.

@agronholm
Copy link
Owner

I was able to do this even on trio (thanks @richardsheridan) but it was a very ugly workaround. Getting this sorted out on trio would therefore be a welcome change.

@richardsheridan
Copy link
Contributor

linking a relevant PR: python-trio/trio#2160

@agronholm
Copy link
Owner

I thought I had a PR ready for this but then I started testing contextvar propagation with BlockingPortal and turns out trio is not behaving correctly there either. I'll have to work some more to make the it work consistently there.

@tiangolo
Copy link
Contributor

I have a PR for this here: #389.

I started the work in Trio in python-trio/trio#2160 with the ultimate objective of having this in AnyIO. 🤓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants