From 2fbd4793c182d20dd34b0ca4319e02b9ff4fdf81 Mon Sep 17 00:00:00 2001 From: Nikolai Ryzhkov Date: Fri, 28 Feb 2025 11:48:12 +0100 Subject: [PATCH 1/3] Reproduce issue #495 --- tests/test_sync.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/test_sync.py b/tests/test_sync.py index 0c67308c..ef5391c1 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -8,7 +8,7 @@ from concurrent.futures import ThreadPoolExecutor from functools import wraps from typing import Any -from unittest import TestCase +from unittest import TestCase, mock import pytest @@ -197,6 +197,22 @@ def test_method(self): assert method.__self__ == instance +@pytest.mark.asyncio +@pytest.mark.parametrize("exc_cls", (RuntimeError, ValueError, Exception)) +async def test_sync_to_async_broken_executor(exc_cls): + """ + Tests sync_to_async catch error in executor and avoid deadlock + """ + with mock.patch.object(ThreadPoolExecutor, "submit") as mock_run: + mock_run.side_effect = exc_cls("Test Error") + async_function = sync_to_async(lambda: None, thread_sensitive=True) + with pytest.raises(exc_cls, match="Test Error"): + await async_function() + with pytest.raises(exc_cls, match="Test Error"): + await async_function() + + + @pytest.mark.asyncio async def test_async_to_sync_to_async(): """ From 30039d1bc27ddc60bfe43e28145dcaec7b44eb70 Mon Sep 17 00:00:00 2001 From: Nikolai Ryzhkov Date: Fri, 28 Feb 2025 11:50:03 +0100 Subject: [PATCH 2/3] Rollback state of SyncToAsync if loop.run_in_executor raises an exception --- asgiref/sync.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/asgiref/sync.py b/asgiref/sync.py index 377075d1..82c8279d 100644 --- a/asgiref/sync.py +++ b/asgiref/sync.py @@ -452,22 +452,29 @@ async def __call__(self, *args: _P.args, **kwargs: _P.kwargs) -> _R: func = context.run task_context: List[asyncio.Task[Any]] = [] - # Run the code in the right thread - exec_coro = loop.run_in_executor( - executor, - functools.partial( - self.thread_handler, - loop, - sys.exc_info(), - task_context, - func, - child, - ), - ) + try: + # Run the code in the right thread + exec_coro = loop.run_in_executor( + executor, + functools.partial( + self.thread_handler, + loop, + sys.exc_info(), + task_context, + func, + child, + ), + ) + except: + _restore_context(context) + self.deadlock_context.set(False) + raise + ret: _R try: ret = await asyncio.shield(exec_coro) except asyncio.CancelledError: + # catch CancelledError only in await cancel_parent = True try: task = task_context[0] From c8ce675b99e8f0c1d54fb9d2506dc71ad0c7775b Mon Sep 17 00:00:00 2001 From: Nikolai Ryzhkov Date: Fri, 28 Feb 2025 12:27:30 +0100 Subject: [PATCH 3/3] Style fix --- asgiref/sync.py | 2 +- tests/test_sync.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/asgiref/sync.py b/asgiref/sync.py index 82c8279d..7f9e6f50 100644 --- a/asgiref/sync.py +++ b/asgiref/sync.py @@ -465,7 +465,7 @@ async def __call__(self, *args: _P.args, **kwargs: _P.kwargs) -> _R: child, ), ) - except: + except Exception: _restore_context(context) self.deadlock_context.set(False) raise diff --git a/tests/test_sync.py b/tests/test_sync.py index ef5391c1..c8253c2b 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -212,7 +212,6 @@ async def test_sync_to_async_broken_executor(exc_cls): await async_function() - @pytest.mark.asyncio async def test_async_to_sync_to_async(): """