From 74b0194afb9acebf3a09b06d9c1abd97cb7917a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Gr=C3=B6nholm?= Date: Wed, 2 Oct 2024 21:59:20 +0300 Subject: [PATCH 1/2] Fixed missing or inconsistent error when acquiring already owned Lock Fixes #798. --- docs/versionhistory.rst | 5 +++++ src/anyio/_backends/_asyncio.py | 13 ++++++++++--- src/anyio/_backends/_trio.py | 18 +++++++++++++++++- tests/test_synchronization.py | 17 +++++++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/docs/versionhistory.rst b/docs/versionhistory.rst index 22ae559c..c71002b8 100644 --- a/docs/versionhistory.rst +++ b/docs/versionhistory.rst @@ -3,6 +3,11 @@ Version history This library adheres to `Semantic Versioning 2.0 `_. +**UNRELEASED** + +- Fixed acquring a lock twice in the same task on asyncio hanging instead of raising a + ``RuntimeError`` (`#798 `_) + **4.6.0** - Dropped support for Python 3.8 diff --git a/src/anyio/_backends/_asyncio.py b/src/anyio/_backends/_asyncio.py index 9342fab8..b4f82f19 100644 --- a/src/anyio/_backends/_asyncio.py +++ b/src/anyio/_backends/_asyncio.py @@ -1731,9 +1731,10 @@ def __init__(self, *, fast_acquire: bool = False) -> None: self._waiters: deque[tuple[asyncio.Task, asyncio.Future]] = deque() async def acquire(self) -> None: + task = cast(asyncio.Task, current_task()) if self._owner_task is None and not self._waiters: await AsyncIOBackend.checkpoint_if_cancelled() - self._owner_task = current_task() + self._owner_task = task # Unless on the "fast path", yield control of the event loop so that other # tasks can run too @@ -1746,7 +1747,9 @@ async def acquire(self) -> None: return - task = cast(asyncio.Task, current_task()) + if self._owner_task == task: + raise RuntimeError("Attempted to acquire an already held Lock") + fut: asyncio.Future[None] = asyncio.Future() item = task, fut self._waiters.append(item) @@ -1762,10 +1765,14 @@ async def acquire(self) -> None: self._waiters.remove(item) def acquire_nowait(self) -> None: + task = cast(asyncio.Task, current_task()) if self._owner_task is None and not self._waiters: - self._owner_task = current_task() + self._owner_task = task return + if self._owner_task == task: + raise RuntimeError("Attempted to acquire an already held Lock") + raise WouldBlock def locked(self) -> bool: diff --git a/src/anyio/_backends/_trio.py b/src/anyio/_backends/_trio.py index de2189ce..c10a72a1 100644 --- a/src/anyio/_backends/_trio.py +++ b/src/anyio/_backends/_trio.py @@ -662,9 +662,19 @@ def __init__(self, *, fast_acquire: bool = False) -> None: self._fast_acquire = fast_acquire self.__original = trio.Lock() + @staticmethod + def _convert_runtime_error_msg(exc: RuntimeError) -> None: + if exc.args == ("attempt to re-acquire an already held Lock",): + exc.args = ("Attempted to acquire an already held Lock",) + async def acquire(self) -> None: if not self._fast_acquire: - await self.__original.acquire() + try: + await self.__original.acquire() + except RuntimeError as exc: + self._convert_runtime_error_msg(exc) + raise + return # This is the "fast path" where we don't let other tasks run @@ -673,12 +683,18 @@ async def acquire(self) -> None: self.__original.acquire_nowait() except trio.WouldBlock: await self.__original._lot.park() + except RuntimeError as exc: + self._convert_runtime_error_msg(exc) + raise def acquire_nowait(self) -> None: try: self.__original.acquire_nowait() except trio.WouldBlock: raise WouldBlock from None + except RuntimeError as exc: + self._convert_runtime_error_msg(exc) + raise def locked(self) -> bool: return self.__original.locked() diff --git a/tests/test_synchronization.py b/tests/test_synchronization.py index c43dbe5a..cc58188b 100644 --- a/tests/test_synchronization.py +++ b/tests/test_synchronization.py @@ -96,6 +96,23 @@ async def try_lock() -> None: assert lock.locked() tg.start_soon(try_lock) + @pytest.mark.parametrize("fast_acquire", [True, False]) + async def test_acquire_twice_async(self, fast_acquire: bool) -> None: + lock = Lock(fast_acquire=fast_acquire) + await lock.acquire() + with pytest.raises( + RuntimeError, match="Attempted to acquire an already held Lock" + ): + await lock.acquire() + + async def test_acquire_twice_sync(self) -> None: + lock = Lock() + lock.acquire_nowait() + with pytest.raises( + RuntimeError, match="Attempted to acquire an already held Lock" + ): + lock.acquire_nowait() + @pytest.mark.parametrize( "release_first", [pytest.param(False, id="releaselast"), pytest.param(True, id="releasefirst")], From b96cdbc7ced34bcec800efd9bebdfdd7638a4f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Gr=C3=B6nholm?= Date: Sat, 12 Oct 2024 14:48:01 +0300 Subject: [PATCH 2/2] Update src/anyio/_backends/_asyncio.py Co-authored-by: Thomas Grainger --- src/anyio/_backends/_asyncio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/anyio/_backends/_asyncio.py b/src/anyio/_backends/_asyncio.py index b4f82f19..ed00dee0 100644 --- a/src/anyio/_backends/_asyncio.py +++ b/src/anyio/_backends/_asyncio.py @@ -1770,7 +1770,7 @@ def acquire_nowait(self) -> None: self._owner_task = task return - if self._owner_task == task: + if self._owner_task is task: raise RuntimeError("Attempted to acquire an already held Lock") raise WouldBlock