mirror of https://github.com/python/cpython
bpo-32734: Fix asyncio.Lock multiple acquire safety issue (GH-5466)
This commit is contained in:
parent
66771422d0
commit
2f79c01493
|
@ -183,16 +183,22 @@ class Lock(_ContextManagerMixin):
|
||||||
|
|
||||||
fut = self._loop.create_future()
|
fut = self._loop.create_future()
|
||||||
self._waiters.append(fut)
|
self._waiters.append(fut)
|
||||||
|
|
||||||
|
# Finally block should be called before the CancelledError
|
||||||
|
# handling as we don't want CancelledError to call
|
||||||
|
# _wake_up_first() and attempt to wake up itself.
|
||||||
try:
|
try:
|
||||||
await fut
|
try:
|
||||||
self._locked = True
|
await fut
|
||||||
return True
|
finally:
|
||||||
|
self._waiters.remove(fut)
|
||||||
except futures.CancelledError:
|
except futures.CancelledError:
|
||||||
if not self._locked:
|
if not self._locked:
|
||||||
self._wake_up_first()
|
self._wake_up_first()
|
||||||
raise
|
raise
|
||||||
finally:
|
|
||||||
self._waiters.remove(fut)
|
self._locked = True
|
||||||
|
return True
|
||||||
|
|
||||||
def release(self):
|
def release(self):
|
||||||
"""Release a lock.
|
"""Release a lock.
|
||||||
|
@ -212,11 +218,17 @@ class Lock(_ContextManagerMixin):
|
||||||
raise RuntimeError('Lock is not acquired.')
|
raise RuntimeError('Lock is not acquired.')
|
||||||
|
|
||||||
def _wake_up_first(self):
|
def _wake_up_first(self):
|
||||||
"""Wake up the first waiter who isn't cancelled."""
|
"""Wake up the first waiter if it isn't done."""
|
||||||
for fut in self._waiters:
|
try:
|
||||||
if not fut.done():
|
fut = next(iter(self._waiters))
|
||||||
fut.set_result(True)
|
except StopIteration:
|
||||||
break
|
return
|
||||||
|
|
||||||
|
# .done() necessarily means that a waiter will wake up later on and
|
||||||
|
# either take the lock, or, if it was cancelled and lock wasn't
|
||||||
|
# taken already, will hit this again and wake up a new waiter.
|
||||||
|
if not fut.done():
|
||||||
|
fut.set_result(True)
|
||||||
|
|
||||||
|
|
||||||
class Event:
|
class Event:
|
||||||
|
|
|
@ -200,6 +200,56 @@ class LockTests(test_utils.TestCase):
|
||||||
self.assertTrue(tb.cancelled())
|
self.assertTrue(tb.cancelled())
|
||||||
self.assertTrue(tc.done())
|
self.assertTrue(tc.done())
|
||||||
|
|
||||||
|
def test_cancel_release_race(self):
|
||||||
|
# Issue 32734
|
||||||
|
# Acquire 4 locks, cancel second, release first
|
||||||
|
# and 2 locks are taken at once.
|
||||||
|
lock = asyncio.Lock(loop=self.loop)
|
||||||
|
lock_count = 0
|
||||||
|
call_count = 0
|
||||||
|
|
||||||
|
async def lockit():
|
||||||
|
nonlocal lock_count
|
||||||
|
nonlocal call_count
|
||||||
|
call_count += 1
|
||||||
|
await lock.acquire()
|
||||||
|
lock_count += 1
|
||||||
|
|
||||||
|
async def lockandtrigger():
|
||||||
|
await lock.acquire()
|
||||||
|
self.loop.call_soon(trigger)
|
||||||
|
|
||||||
|
def trigger():
|
||||||
|
t1.cancel()
|
||||||
|
lock.release()
|
||||||
|
|
||||||
|
t0 = self.loop.create_task(lockandtrigger())
|
||||||
|
t1 = self.loop.create_task(lockit())
|
||||||
|
t2 = self.loop.create_task(lockit())
|
||||||
|
t3 = self.loop.create_task(lockit())
|
||||||
|
|
||||||
|
# First loop acquires all
|
||||||
|
test_utils.run_briefly(self.loop)
|
||||||
|
self.assertTrue(t0.done())
|
||||||
|
|
||||||
|
# Second loop calls trigger
|
||||||
|
test_utils.run_briefly(self.loop)
|
||||||
|
# Third loop calls cancellation
|
||||||
|
test_utils.run_briefly(self.loop)
|
||||||
|
|
||||||
|
# Make sure only one lock was taken
|
||||||
|
self.assertEqual(lock_count, 1)
|
||||||
|
# While 3 calls were made to lockit()
|
||||||
|
self.assertEqual(call_count, 3)
|
||||||
|
self.assertTrue(t1.cancelled() and t2.done())
|
||||||
|
|
||||||
|
# Cleanup the task that is stuck on acquire.
|
||||||
|
t3.cancel()
|
||||||
|
test_utils.run_briefly(self.loop)
|
||||||
|
self.assertTrue(t3.cancelled())
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
def test_finished_waiter_cancelled(self):
|
def test_finished_waiter_cancelled(self):
|
||||||
lock = asyncio.Lock(loop=self.loop)
|
lock = asyncio.Lock(loop=self.loop)
|
||||||
|
|
||||||
|
|
|
@ -602,6 +602,7 @@ Milton L. Hankins
|
||||||
Stephen Hansen
|
Stephen Hansen
|
||||||
Barry Hantman
|
Barry Hantman
|
||||||
Lynda Hardman
|
Lynda Hardman
|
||||||
|
Bar Harel
|
||||||
Derek Harland
|
Derek Harland
|
||||||
Jason Harper
|
Jason Harper
|
||||||
David Harrigan
|
David Harrigan
|
||||||
|
|
|
@ -0,0 +1,2 @@
|
||||||
|
Fixed ``asyncio.Lock()`` safety issue which allowed acquiring and locking
|
||||||
|
the same lock multiple times, without it being free. Patch by Bar Harel.
|
Loading…
Reference in New Issue