Ability to change contextvars in sync dependencies

[smagafurov]

Sync dependencies now run in thread pool with starlette.concurrency.run_in_threadpool.

run_in_threadpool run in the context copy (contextvars.copy_context().run(...))

async def run_in_threadpool(
    func: typing.Callable, *args: typing.Any, **kwargs: typing.Any
) -> typing.Any:
    loop = asyncio.get_event_loop()
    if contextvars is not None:  # pragma: no cover
        # Ensure we run in the same context
        child = functools.partial(func, *args, **kwargs)
        context = contextvars.copy_context()  # <------------------ here
        func = context.run
        args = (child,)
    elif kwargs:  # pragma: no cover
        # loop.run_in_executor doesn't accept 'kwargs', so bind them in here
        func = functools.partial(func, **kwargs)
    return await loop.run_in_executor(None, func, *args)

But if we change contextvars inside dependency (func), this changes are lost. We don't see this changes in route func.

But I want to see this changes :)

May be we can propagate (copy) contextvars changes to main thread contextvars after dependency finish?

https://github.com/tiangolo/fastapi/blob/d91b2b3ee8fa428544769aba664aedbf9ed6511a/fastapi/dependencies/utils.py#L521

[dmontagu]

I think this should be a feature request for starlette rather than FastAPI; it might be worth creating an issue there. @tomchristie might know of a good pattern for accomplishing this (or whether it is even possible), or might be able to provide reasons why you should avoid this pattern.

[lovelydinosaur]

(No idea)

[smagafurov]

I think this should be a feature request for starlette rather than FastAPI; it might be worth creating an issue there. @tomchristie might know of a good pattern for accomplishing this (or whether it is even possible), or might be able to provide reasons why you should avoid this pattern.

dependency - is a subject of fastapi (not starlette)

Logically dependency should have the same context as route handler.

If dependency is async - it works correct (have the same context).

Sync dependency and async dependency have the same logic.

So sync dependency should have the same context too (logically the same).

There is problem with sync dependency only, because of technical realization of sync dependency (they called with run_in_threadpool). So we should think about this problem in fatsapi (not in starlette)

[tiangolo]

Oh, that's a heavy hardcore subject 😅

So, as sync (normal def) dependencies run in a threadpool, if you set a contextvar value inside the sync function, the context var will be set only for that thread. And once execution comes out of that thread and then goes to the path operation, it sees the previous value of the contextvar, and also, the sync function of the path operation also runs in a threadpool, so if it changes the contextvar, that only applies to that function (running in that thread).

---

✨ But, there's a way to solve it/workaround it. ✨

You can create an async dependency that only sets a value in the context var, and in the exit code resets it (so that the next request sees the default again). The value you set in that context var wouldn't be a final scalar value, but a mutable (a dict or an object). Then, in the sync dependency you would only read the contextvar (that was set in the async dependency). Because you are not setting a value, you can inherit the value in the thread. But then you can "mutate" it, you can set stuff to the object form the contextvar. And because you are not resetting the contextvar but just putting stuff in it, mutating it in place, the path operation sync function (that also runs in the threadpool, in one thread) will be able to see the same value for the contextvar, but now mutated by the sync dependency.

After that, the sync dependency's exit code will still see the same contextvar value, because nothing has changed it, since the async dependency set it. The contextvar value was mutated, setting values to attributes in the object or similar, at the beginning of the same sync dependency, but as it is the same contextvar value, it can still see it and execute its exit code using the mutated value.

Then, after that, the async dependency would see again the unmodified (but mutated) contextvar that it set in the beginning, and then it can execute its exit code to reset the value for the context var, for the next request.

The next question would probably be: if the value of the contextvar that is set in the async dependency is then mutated in place by different threads in the threadpool, by the sync dependency and possibly the path operation sync function (or any other sync sub-dependency), how can it be safe? And the answer is that the async dependency set a value for the current request, it makes it independent of any other request. And all those possible mutations to the value in the threadpool by the sync dependency (and any sub-dependency) and possibly by the path operation are all run sequentially. The sync dependency is never executed at the same time as the sync path operation function for the current request. So, effectively, the contextvar is associated with each request, which is what we are dealing with.

If you want to read more details about it, check the docs for using FastAPI with Peewee: https://fastapi.tiangolo.com/advanced/sql-databases-peewee/

Pewee doesn't have a way to let you handle connections/sessions yourself. It does that in thread locals. So, to be able to use it correctly, it's necessary to monkey patch a couple of places to make it use contextvars, and then all the setup of those contextvars is just as I described above.

---

Given the amount of 👍 in this issue (and my increase in white hairs after all that Peewee adventure 😅 ) I think this is worth documenting in the advanced user guide.

But still, I wanted to give a "quick" overview here for those you who are reading.

[smagafurov]

There is something in Django's asgiref library about contextvars restoring from threadpool: https://github.com/django/asgiref/blob/master/asgiref/sync.py#L18

[adriangb]

FYI Kludex/starlette#1258 adapts asgiref's implementation for Starlette and, as far as I can tell, completely solves this issue (that is if there is agreement that copying back changes to the context is actually the right thing to do here).

I ran that branch against 58ab733 and all tests here still pass.