From 7e1cc8ce730fafa3f02ad0d11e0692b62ab4ba3b Mon Sep 17 00:00:00 2001 From: Mathieu Sornay Date: Fri, 9 Jun 2017 22:17:40 +0200 Subject: [PATCH] Fix waiter cancellation in asyncio.Lock (#1031) Avoid a deadlock when the waiter who is about to take the lock is cancelled Issue #27585 --- Lib/asyncio/locks.py | 17 ++++++++++++----- Lib/test/test_asyncio/test_locks.py | 22 ++++++++++++++++++++++ Misc/NEWS | 3 +++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/Lib/asyncio/locks.py b/Lib/asyncio/locks.py index deefc938ecfb01..92661830a06228 100644 --- a/Lib/asyncio/locks.py +++ b/Lib/asyncio/locks.py @@ -176,6 +176,10 @@ def acquire(self): yield from fut self._locked = True return True + except futures.CancelledError: + if not self._locked: + self._wake_up_first() + raise finally: self._waiters.remove(fut) @@ -192,14 +196,17 @@ def release(self): """ if self._locked: self._locked = False - # Wake up the first waiter who isn't cancelled. - for fut in self._waiters: - if not fut.done(): - fut.set_result(True) - break + self._wake_up_first() else: raise RuntimeError('Lock is not acquired.') + def _wake_up_first(self): + """Wake up the first waiter who isn't cancelled.""" + for fut in self._waiters: + if not fut.done(): + fut.set_result(True) + break + class Event: """Asynchronous equivalent to threading.Event. diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index 152948c8138975..c85e8b1a32f73a 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -176,6 +176,28 @@ def lockit(name, blocker): self.assertTrue(tb.cancelled()) self.assertTrue(tc.done()) + def test_finished_waiter_cancelled(self): + lock = asyncio.Lock(loop=self.loop) + + ta = asyncio.Task(lock.acquire(), loop=self.loop) + test_utils.run_briefly(self.loop) + self.assertTrue(lock.locked()) + + tb = asyncio.Task(lock.acquire(), loop=self.loop) + test_utils.run_briefly(self.loop) + self.assertEqual(len(lock._waiters), 1) + + # Create a second waiter, wake up the first, and cancel it. + # Without the fix, the second was not woken up. + tc = asyncio.Task(lock.acquire(), loop=self.loop) + lock.release() + tb.cancel() + test_utils.run_briefly(self.loop) + + self.assertTrue(lock.locked()) + self.assertTrue(ta.done()) + self.assertTrue(tb.cancelled()) + def test_release_not_acquired(self): lock = asyncio.Lock(loop=self.loop) diff --git a/Misc/NEWS b/Misc/NEWS index 78ec9f0ccae64a..af50b0f96b568b 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -56,6 +56,9 @@ Extension Modules Library ------- +- bpo-27585: Fix waiter cancellation in asyncio.Lock. + Patch by Mathieu Sornay. + - bpo-30418: On Windows, subprocess.Popen.communicate() now also ignore EINVAL on stdin.write() if the child process is still running but closed the pipe.