Skip to content

Commit 7e4cf8e

Browse files
bharel1st1
authored andcommitted
[3.6] bpo-32734: Fix asyncio.Lock multiple acquire safety issue (GH-5466) (#5502)
(cherry picked from commit d41e9e0)
1 parent fbf8e82 commit 7e4cf8e

File tree

4 files changed

+75
-10
lines changed

4 files changed

+75
-10
lines changed

Lib/asyncio/locks.py

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,22 @@ def acquire(self):
172172

173173
fut = self._loop.create_future()
174174
self._waiters.append(fut)
175+
176+
# Finally block should be called before the CancelledError
177+
# handling as we don't want CancelledError to call
178+
# _wake_up_first() and attempt to wake up itself.
175179
try:
176-
yield from fut
177-
self._locked = True
178-
return True
180+
try:
181+
yield from fut
182+
finally:
183+
self._waiters.remove(fut)
179184
except futures.CancelledError:
180185
if not self._locked:
181186
self._wake_up_first()
182187
raise
183-
finally:
184-
self._waiters.remove(fut)
188+
189+
self._locked = True
190+
return True
185191

186192
def release(self):
187193
"""Release a lock.
@@ -201,11 +207,17 @@ def release(self):
201207
raise RuntimeError('Lock is not acquired.')
202208

203209
def _wake_up_first(self):
204-
"""Wake up the first waiter who isn't cancelled."""
205-
for fut in self._waiters:
206-
if not fut.done():
207-
fut.set_result(True)
208-
break
210+
"""Wake up the first waiter if it isn't done."""
211+
try:
212+
fut = next(iter(self._waiters))
213+
except StopIteration:
214+
return
215+
216+
# .done() necessarily means that a waiter will wake up later on and
217+
# either take the lock, or, if it was cancelled and lock wasn't
218+
# taken already, will hit this again and wake up a new waiter.
219+
if not fut.done():
220+
fut.set_result(True)
209221

210222

211223
class Event:

Lib/test/test_asyncio/test_locks.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,56 @@ def lockit(name, blocker):
176176
self.assertTrue(tb.cancelled())
177177
self.assertTrue(tc.done())
178178

179+
def test_cancel_release_race(self):
180+
# Issue 32734
181+
# Acquire 4 locks, cancel second, release first
182+
# and 2 locks are taken at once.
183+
lock = asyncio.Lock(loop=self.loop)
184+
lock_count = 0
185+
call_count = 0
186+
187+
async def lockit():
188+
nonlocal lock_count
189+
nonlocal call_count
190+
call_count += 1
191+
await lock.acquire()
192+
lock_count += 1
193+
194+
async def lockandtrigger():
195+
await lock.acquire()
196+
self.loop.call_soon(trigger)
197+
198+
def trigger():
199+
t1.cancel()
200+
lock.release()
201+
202+
t0 = self.loop.create_task(lockandtrigger())
203+
t1 = self.loop.create_task(lockit())
204+
t2 = self.loop.create_task(lockit())
205+
t3 = self.loop.create_task(lockit())
206+
207+
# First loop acquires all
208+
test_utils.run_briefly(self.loop)
209+
self.assertTrue(t0.done())
210+
211+
# Second loop calls trigger
212+
test_utils.run_briefly(self.loop)
213+
# Third loop calls cancellation
214+
test_utils.run_briefly(self.loop)
215+
216+
# Make sure only one lock was taken
217+
self.assertEqual(lock_count, 1)
218+
# While 3 calls were made to lockit()
219+
self.assertEqual(call_count, 3)
220+
self.assertTrue(t1.cancelled() and t2.done())
221+
222+
# Cleanup the task that is stuck on acquire.
223+
t3.cancel()
224+
test_utils.run_briefly(self.loop)
225+
self.assertTrue(t3.cancelled())
226+
227+
228+
179229
def test_finished_waiter_cancelled(self):
180230
lock = asyncio.Lock(loop=self.loop)
181231

Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,7 @@ Milton L. Hankins
589589
Stephen Hansen
590590
Barry Hantman
591591
Lynda Hardman
592+
Bar Harel
592593
Derek Harland
593594
Jason Harper
594595
David Harrigan
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fixed ``asyncio.Lock()`` safety issue which allowed acquiring and locking
2+
the same lock multiple times, without it being free. Patch by Bar Harel.

0 commit comments

Comments
 (0)