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

[3.8 regression] Deadlock with async_to_syncsync_to_asyncasync_to_synccreate_tasksync_to_async #492

Open
andersk opened this issue Feb 5, 2025 · 2 comments · May be fixed by #494

Comments

@andersk
Copy link
Contributor

andersk commented Feb 5, 2025

This code, minimized from the Zulip server, worked in asgiref 3.7.2, but deadlocks in 3.8.0, 3.8.1, and current main (abc69a0). A Git bisection implicates d920c3c as the first broken commit:

import asyncio

from asgiref.sync import async_to_sync, sync_to_async


async def inner() -> asyncio.Task[None]:
    return asyncio.create_task(sync_to_async(print)("inner"))


async def main() -> None:
    task = await sync_to_async(async_to_sync(inner))()
    await task


async_to_sync(main)()
andersk added a commit to andersk/asgiref that referenced this issue Feb 5, 2025
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/asgiref that referenced this issue Feb 13, 2025
A CurrentThreadExecutor may terminate with work still remaining in its
queue, or new work may be submitted later.  We previously discarded
remaining work, leading to deadlocks, and raised an error on
submitting late work.  But if there’s another CurrentThreadExecutor
for the same thread below us on the stack, we should instead migrate
the extra work there to allow it to eventually run.

Doing this in a thread-safe way requires replacing the queue.Queue
abstraction with collections.deque and
threading.ConditionVariable (the same primitives used to implement
queue.Queue).

Fixes django#491; fixes django#492.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/asgiref that referenced this issue Feb 13, 2025
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/asgiref that referenced this issue Feb 14, 2025
A CurrentThreadExecutor may terminate with work still remaining in its
queue, or new work may be submitted later.  We previously discarded
remaining work, leading to deadlocks, and raised an error on
submitting late work.  But if there’s another CurrentThreadExecutor
for the same thread below us on the stack, we should instead migrate
the extra work there to allow it to eventually run.

Doing this in a thread-safe way requires replacing the queue.Queue
abstraction with collections.deque and
threading.ConditionVariable (the same primitives used to implement
queue.Queue).

Fixes django#491; fixes django#492.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/asgiref that referenced this issue Feb 14, 2025
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@bartekgruszka
Copy link

Any updates on this bug? Thanks!

@carltongibson
Copy link
Member

@bartekgruszka Trying out the branch from #494 would be helpful. (See related discussions from three of you want to dig deeper.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants