Skip to content

Commit 141d5a0

Browse files
authored
AsyncRLock: don't swallow Lock.acquire errors (#944)
Addressing point 2 ("Missing error handling in `_acquire_non_blocking`") raised in #937. It's not clear if or when `asyncio.Lock.acquire()` would raise an exception. But should it, our wrapping `AsyncRLock `should not silently swallow it and worse pretend the lock was successfully acquired.
1 parent 1e5c7f9 commit 141d5a0

File tree

2 files changed

+71
-4
lines changed

2 files changed

+71
-4
lines changed

src/neo4j/_async_compat/concurrency.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,14 @@ async def _acquire_non_blocking(self, me):
100100
# Hence, we flag this task as cancelled again, so that the next
101101
# `await` will raise the CancelledError.
102102
asyncio.current_task().cancel()
103-
if task.done() and task.exception() is None:
104-
self._owner = me
105-
self._count = 1
106-
return True
103+
if task.done():
104+
exception = task.exception()
105+
if exception is None:
106+
self._owner = me
107+
self._count = 1
108+
return True
109+
else:
110+
raise exception
107111
task.cancel()
108112
return False
109113

tests/unit/mixed/async_compat/test_concurrency.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,69 @@ async def waiter():
168168
assert lock.locked() # waiter_non_blocking still owns it!
169169

170170

171+
@pytest.mark.asyncio
172+
async def test_async_r_lock_acquire_non_blocking_exception(mocker):
173+
lock = AsyncRLock()
174+
exc = RuntimeError("it broke!")
175+
176+
assert not lock.locked()
177+
178+
# Not sure asyncio.Lock.acquire is even fallible, but should it be or ever
179+
# become, our AsyncRLock should re-raise the exception.
180+
acquire_mock = mocker.patch("asyncio.Lock.acquire", autospec=True,
181+
side_effect=asyncio.Lock.acquire)
182+
awaits = 0
183+
184+
async def blocker():
185+
nonlocal awaits
186+
187+
assert awaits == 0
188+
awaits += 1
189+
assert await lock.acquire()
190+
191+
assert awaits == 1
192+
awaits += 1
193+
await asyncio.sleep(0)
194+
195+
assert awaits == 3
196+
awaits += 1
197+
await asyncio.sleep(0)
198+
199+
assert awaits == 5
200+
awaits += 1
201+
await asyncio.sleep(0)
202+
203+
assert awaits == 7
204+
lock.release()
205+
206+
async def waiter_non_blocking():
207+
nonlocal awaits
208+
nonlocal acquire_mock
209+
210+
assert awaits == 2
211+
acquire_mock.side_effect = exc
212+
coro = lock.acquire(blocking=False)
213+
fut = asyncio.ensure_future(coro)
214+
assert not fut.done()
215+
awaits += 1
216+
await asyncio.sleep(0)
217+
218+
assert awaits == 4
219+
assert not fut.done()
220+
awaits += 1
221+
await asyncio.sleep(0)
222+
223+
assert awaits == 6
224+
assert fut.done()
225+
assert fut.exception() is exc
226+
awaits += 1
227+
228+
229+
assert not lock.locked()
230+
await asyncio.gather(blocker(), waiter_non_blocking())
231+
assert not lock.locked()
232+
233+
171234
@pytest.mark.parametrize("waits", range(1, 10))
172235
@pytest.mark.asyncio
173236
async def test_async_r_lock_acquire_cancellation(waits):

0 commit comments

Comments
 (0)