gh-90155: Fix bug in asyncio.Semaphore and strengthen FIFO guarantee (GH-93222)

The main problem was that an unluckily timed task cancellation could cause
the semaphore to be stuck. There were also doubts about strict FIFO ordering
of tasks allowed to pass.

The Semaphore implementation was rewritten to be more similar to Lock.
Many tests for edge cases (including cancellation) were added.
(cherry picked from commit 24e0379624)

Co-authored-by: Cyker Way <cykerway@gmail.com>
This commit is contained in:
Miss Islington (bot) 2022-09-22 09:58:35 -07:00 committed by GitHub
parent fb87aaafba
commit 773dbb9e3a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 143 additions and 22 deletions

View file

@ -5,6 +5,7 @@ from unittest import mock
import re
import asyncio
import collections
STR_RGX_REPR = (
r'^<(?P<class>.*?) object at (?P<address>.*?)'
@ -774,6 +775,9 @@ class SemaphoreTests(unittest.IsolatedAsyncioTestCase):
self.assertTrue('waiters' not in repr(sem))
self.assertTrue(RGX_REPR.match(repr(sem)))
if sem._waiters is None:
sem._waiters = collections.deque()
sem._waiters.append(mock.Mock())
self.assertTrue('waiters:1' in repr(sem))
self.assertTrue(RGX_REPR.match(repr(sem)))
@ -842,6 +846,7 @@ class SemaphoreTests(unittest.IsolatedAsyncioTestCase):
sem.release()
self.assertEqual(2, sem._value)
await asyncio.sleep(0)
await asyncio.sleep(0)
self.assertEqual(0, sem._value)
self.assertEqual(3, len(result))
@ -884,6 +889,7 @@ class SemaphoreTests(unittest.IsolatedAsyncioTestCase):
t2.cancel()
sem.release()
await asyncio.sleep(0)
await asyncio.sleep(0)
num_done = sum(t.done() for t in [t3, t4])
self.assertEqual(num_done, 1)
@ -904,9 +910,32 @@ class SemaphoreTests(unittest.IsolatedAsyncioTestCase):
t1.cancel()
sem.release()
await asyncio.sleep(0)
await asyncio.sleep(0)
self.assertTrue(sem.locked())
self.assertTrue(t2.done())
async def test_acquire_no_hang(self):
sem = asyncio.Semaphore(1)
async def c1():
async with sem:
await asyncio.sleep(0)
t2.cancel()
async def c2():
async with sem:
self.assertFalse(True)
t1 = asyncio.create_task(c1())
t2 = asyncio.create_task(c2())
r1, r2 = await asyncio.gather(t1, t2, return_exceptions=True)
self.assertTrue(r1 is None)
self.assertTrue(isinstance(r2, asyncio.CancelledError))
await asyncio.wait_for(sem.acquire(), timeout=1.0)
def test_release_not_acquired(self):
sem = asyncio.BoundedSemaphore()
@ -945,6 +974,77 @@ class SemaphoreTests(unittest.IsolatedAsyncioTestCase):
result
)
async def test_acquire_fifo_order_2(self):
sem = asyncio.Semaphore(1)
result = []
async def c1(result):
await sem.acquire()
result.append(1)
return True
async def c2(result):
await sem.acquire()
result.append(2)
sem.release()
await sem.acquire()
result.append(4)
return True
async def c3(result):
await sem.acquire()
result.append(3)
return True
t1 = asyncio.create_task(c1(result))
t2 = asyncio.create_task(c2(result))
t3 = asyncio.create_task(c3(result))
await asyncio.sleep(0)
sem.release()
sem.release()
tasks = [t1, t2, t3]
await asyncio.gather(*tasks)
self.assertEqual([1, 2, 3, 4], result)
async def test_acquire_fifo_order_3(self):
sem = asyncio.Semaphore(0)
result = []
async def c1(result):
await sem.acquire()
result.append(1)
return True
async def c2(result):
await sem.acquire()
result.append(2)
return True
async def c3(result):
await sem.acquire()
result.append(3)
return True
t1 = asyncio.create_task(c1(result))
t2 = asyncio.create_task(c2(result))
t3 = asyncio.create_task(c3(result))
await asyncio.sleep(0)
t1.cancel()
await asyncio.sleep(0)
sem.release()
sem.release()
tasks = [t1, t2, t3]
await asyncio.gather(*tasks, return_exceptions=True)
self.assertEqual([2, 3], result)
class BarrierTests(unittest.IsolatedAsyncioTestCase):