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

async_to_sync does not use the correct thread when used as decorator #438

Open
germaniuss opened this issue Jan 19, 2024 · 3 comments
Open

Comments

@germaniuss
Copy link
Contributor

germaniuss commented Jan 19, 2024

When using a function decorated with @async_to_sync inside of a function decorated with @sync_to_async a new thread is spawned instead of reusing the same thread and event loop. This issue does not happen when doing async_to_sync(coroutine)().

This is due to the fact that the AsyncToSync sets main_event_loop and main_event_loop_pid on __init__ and not on __call__ and SyncToAsync only sets main_event_loop and main_event_loop_pid on __call__.

The easy way to solve this is moving the initialization of self.main_event_loop and self.main_event_loop_pid from __init__ to __call__ in AsyncToSync (#439 fixes the issue)

Consider this example on why this is relevant:

import asyncio
from asgiref.sync import sync_to_async, async_to_sync
import threading

# this function cannot be changed
def lib_call(callback):
    callback.callback()

class Callback:
    def callback():
        async def coro():
            print(f"Callback thread: {threading.get_ident()}")
            # async code here
        async_to_sync(coro)()
 
class CallbackDecorated:
    @async_to_sync   
    async def callback():
        print(f"Callback decorated thread: {threading.get_ident()}")
        # async code here
    
@sync_to_async
def blocking_func():
    print(f"Blocking func thread: {threading.get_ident()}")
    # this uses the outer event loop
    lib_call(Callback())
    # this spawns a new thread and new event loop for the callback
    lib_call(CallbackDecorated())

async def main():
    try:
        async with asyncio.TaskGroup() as tg:
            task0 = tg.create_task(blocking_func())
    except Exception as exc:
        pass
    
print(f"Main thread: {threading.get_ident()}")
asyncio.run(main())

The output is:

Main thread: 140545354248192
Blocking func thread: 140545298830912
Callback thread: 140545354248192
Callback decorated thread: 140544949696064

In this example calling async_to_sync in blocking_func is a bit akward, therefore our only option is to modify the callback method. This however is also akward since we have to create an auxiliary coroutine for it to use the correct thread. The best syntax would be to use the @async_to_sync decorator that is however not thread safe.

@Ruman-12
Copy link

Hey @germaniuss i would like work on this issue.
could you please assign me this issue?

@andrewgodwin
Copy link
Member

This is interesting because I personally do a lot of async_to_sync(coroutine)() style code so I probably didn't notice this myself!
PR looks good - one linting issue that needs fixing and I can land it.

@germaniuss
Copy link
Contributor Author

germaniuss commented Jan 22, 2024

This is interesting because I personally do a lot of async_to_sync(coroutine)() style code so I probably didn't notice this myself! PR looks good - one linting issue that needs fixing and I can land it.

There is actually one issue I spotted in the PR. In case I run the new decorated function in a thread not started by SyncToAsync, the AsyncToSync object would not have self.main_event_loop initialized, I fixed that issue by moving the check for if there is a running event loop back to __init__ (which made no sense to check in __call__ anyways).

Also fixed the linter issue.

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

Successfully merging a pull request may close this issue.

3 participants