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

sync_to_async(..., thread_sensitive=True) calls in Django class-based async view run in different threads #368

Closed
sh-at-cs opened this issue Jan 11, 2023 · 3 comments

Comments

@sh-at-cs
Copy link

sh-at-cs commented Jan 11, 2023

Description / example

I guess this is similar to #357 except it doesn't even require complicated nesting or anything: Calling sync_to_async(..., thread_sensitive=True)-wrapped code from within an async method in a class-based Django view like so

def f():
    print("enter f()")
    print(f"sync_to_async()-wrapped f() in thread: {threading.get_ident()}")
    time.sleep(10)
    print("exit f()")


class MyView(View):
    async def get(self, *a, **kw):
        print(f"request handled in thread: {threading.get_ident()}")
        await sync_to_async(f, thread_sensitive=True)()
        return HttpResponse("")

will also cause the wrapped code to run in separate threads:

request handled in thread: 140638173488960
enter f()
sync_to_async()-wrapped f() in thread: 140638133942016
request handled in thread: 140638173488960
enter f()
sync_to_async()-wrapped f() in thread: 140638125287168
exit f()
exit f()

This is despite the docs claiming it should always be a shared thread:

Thread-sensitive mode is quite special, and does a lot of work to run all functions in the same thread. Note, though, that it relies on usage of async_to_sync() above it in the stack to correctly run things on the main thread. If you use asyncio.run() or similar, it will fall back to running thread-sensitive functions in a single, shared thread, but this will not be the main thread.

Minimal reproducible example

A full minimal reproducible example repo demonstrating this can be found here (see there for instructions to reproduce): https://github.com/sh-at-cs/django-sync-to-async-thread-bug

Comments

So I wonder: Is this a bug, did I misread the docs, or should something be changed in them to reflect the actual behavior?

Also let me know if this issue should be addressed to the main Django project instead of asgiref...

Version information

Tested with Django 4.1.5, asgiref 3.6.0.

@andrewgodwin
Copy link
Member

It's only meant to be the same thread for a given request, because Django sets a different "context" for each request which we respect. Does it do the right thing if called in the same request twice?

@sh-at-cs
Copy link
Author

Oh I see, thanks!
Yes, it does end up in one and the same thread for calls that happen while handling the same request.
Are there any docs on this I missed? And if not, would it be worth it to open another ticket on Django's tracker about mentioning that it applies per request in the docs I cited?

@andrewgodwin
Copy link
Member

I don't know if there's docs that you missed that might have indicated it but put it this way - more docs and mentioning it where you read it would be good either way!

@sh-at-cs sh-at-cs closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants