From 18adc7c1fed39adbace0c892aef8552132e6b34f Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Sun, 19 Dec 2021 08:28:37 +0200 Subject: [PATCH 01/10] Fix semaphore fifo order --- Lib/asyncio/locks.py | 15 +++++++++------ Lib/test/test_asyncio/test_locks.py | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/Lib/asyncio/locks.py b/Lib/asyncio/locks.py index 4fef64e3921e17..654b7a1cee97c4 100644 --- a/Lib/asyncio/locks.py +++ b/Lib/asyncio/locks.py @@ -6,6 +6,7 @@ from . import exceptions from . import mixins +from . import tasks class _ContextManagerMixin: @@ -350,6 +351,7 @@ def __init__(self, value=1, *, loop=mixins._marker): raise ValueError("Semaphore initial value must be >= 0") self._value = value self._waiters = collections.deque() + self._wakeup_scheduled = False def __repr__(self): res = super().__repr__() @@ -363,6 +365,7 @@ def _wake_up_next(self): waiter = self._waiters.popleft() if not waiter.done(): waiter.set_result(None) + self._wakeup_scheduled = True return def locked(self): @@ -378,16 +381,16 @@ async def acquire(self): called release() to make it larger than 0, and then return True. """ - while self._value <= 0: + while self._wakeup_scheduled or self._value <= 0: fut = self._get_loop().create_future() self._waiters.append(fut) try: await fut - except: - # See the similar code in Queue.get. - fut.cancel() - if self._value > 0 and not fut.cancelled(): - self._wake_up_next() + self._wakeup_scheduled = False + except exceptions.CancelledError: + self._wakeup_scheduled = False + # The future was cancelled, wake up next waiter if any + self._wake_up_next() raise self._value -= 1 return True diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index b2492c1acfecef..41acdf1e1b9e98 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -965,6 +965,32 @@ def test_release_no_waiters(self): sem.release() self.assertFalse(sem.locked()) + def test_acquire_fifo_order(self): + sem = asyncio.Semaphore(1) + result = [] + + async def coro(tag): + await sem.acquire() + result.append(f'{tag}_1') + await asyncio.sleep(0.01) + sem.release() + + await sem.acquire() + result.append(f'{tag}_2') + await asyncio.sleep(0.01) + sem.release() + + t1 = self.loop.create_task(coro('c1')) + t2 = self.loop.create_task(coro('c2')) + t3 = self.loop.create_task(coro('c3')) + + race_tasks = [t1, t2, t3] + self.loop.run_until_complete(asyncio.gather(*race_tasks)) + self.assertEqual( + ['c1_1', 'c2_1', 'c3_1', 'c1_2', 'c2_2', 'c3_2'], + result + ) + if __name__ == '__main__': unittest.main() From 915ae00ae389df761be0e9306c1e289c0cf0f41e Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Sun, 19 Dec 2021 10:47:40 +0200 Subject: [PATCH 02/10] Strip IsolatedAsyncioTestCase frames from reported stacktraces --- Lib/unittest/async_case.py | 1 + .../next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst diff --git a/Lib/unittest/async_case.py b/Lib/unittest/async_case.py index d8bfaf6b67e004..19debde35d82a1 100644 --- a/Lib/unittest/async_case.py +++ b/Lib/unittest/async_case.py @@ -4,6 +4,7 @@ from .case import TestCase +__unittest = True class IsolatedAsyncioTestCase(TestCase): diff --git a/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst b/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst new file mode 100644 index 00000000000000..133b267c09dcfa --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst @@ -0,0 +1,2 @@ +Strip :class:`unittest.IsolatedAsyncioTestCase` frames from reported +stacktraces. From 016742dcb0ff98a09ede5baa2e998a9b33fcdbf6 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Sun, 19 Dec 2021 10:54:26 +0200 Subject: [PATCH 03/10] Update Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> --- .../next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst b/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst index 133b267c09dcfa..7d11d20d94e8a3 100644 --- a/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst +++ b/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst @@ -1,2 +1,2 @@ -Strip :class:`unittest.IsolatedAsyncioTestCase` frames from reported +Strip :class:`unittest.IsolatedAsyncioTestCase` stack frames from reported stacktraces. From 0d214ae0661ac4d9155895832629b085aa89f655 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 15 Mar 2022 18:25:34 +0200 Subject: [PATCH 04/10] Fix test --- Lib/asyncio/locks.py | 1 + Lib/test/test_asyncio/test_locks.py | 11 +++++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/asyncio/locks.py b/Lib/asyncio/locks.py index f79d5d46a0b7e0..d35e4735b3886b 100644 --- a/Lib/asyncio/locks.py +++ b/Lib/asyncio/locks.py @@ -382,6 +382,7 @@ async def acquire(self): self._waiters.append(fut) try: await fut + # reset _wakeup_scheduled *after* waiting for a future self._wakeup_scheduled = False except exceptions.CancelledError: self._wakeup_scheduled = False diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index afcd7cba055f39..920b3b5717a2cc 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -917,7 +917,7 @@ async def test_release_no_waiters(self): sem.release() self.assertFalse(sem.locked()) - def test_acquire_fifo_order(self): + async def test_acquire_fifo_order(self): sem = asyncio.Semaphore(1) result = [] @@ -932,12 +932,11 @@ async def coro(tag): await asyncio.sleep(0.01) sem.release() - t1 = self.loop.create_task(coro('c1')) - t2 = self.loop.create_task(coro('c2')) - t3 = self.loop.create_task(coro('c3')) + async with asyncio.TaskGroup() as tg: + tg.create_task(coro('c1')) + tg.create_task(coro('c2')) + tg.create_task(coro('c3')) - race_tasks = [t1, t2, t3] - self.loop.run_until_complete(asyncio.gather(*race_tasks)) self.assertEqual( ['c1_1', 'c2_1', 'c3_1', 'c1_2', 'c2_2', 'c3_2'], result From 8998363e4dc5eb86f6392edb46671fe343d570ed Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 15 Mar 2022 18:27:03 +0200 Subject: [PATCH 05/10] Cleanup --- Lib/unittest/async_case.py | 1 - .../next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst | 2 -- 2 files changed, 3 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst diff --git a/Lib/unittest/async_case.py b/Lib/unittest/async_case.py index 186f2ca1a5bcc6..25adc3deff63d1 100644 --- a/Lib/unittest/async_case.py +++ b/Lib/unittest/async_case.py @@ -5,7 +5,6 @@ from .case import TestCase -__unittest = True class IsolatedAsyncioTestCase(TestCase): # Names intentionally have a long prefix diff --git a/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst b/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst deleted file mode 100644 index 7d11d20d94e8a3..00000000000000 --- a/Misc/NEWS.d/next/Library/2021-12-19-10-47-24.bpo-46128.Qv3EK1.rst +++ /dev/null @@ -1,2 +0,0 @@ -Strip :class:`unittest.IsolatedAsyncioTestCase` stack frames from reported -stacktraces. From d5e5b7b1f36669feca8290a078e3ae36802b754e Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 15 Mar 2022 18:32:19 +0200 Subject: [PATCH 06/10] Add news --- .../NEWS.d/next/Library/2022-03-15-18-32-12.bpo-45997.4n2aVU.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2022-03-15-18-32-12.bpo-45997.4n2aVU.rst diff --git a/Misc/NEWS.d/next/Library/2022-03-15-18-32-12.bpo-45997.4n2aVU.rst b/Misc/NEWS.d/next/Library/2022-03-15-18-32-12.bpo-45997.4n2aVU.rst new file mode 100644 index 00000000000000..40d8504e5a946b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-03-15-18-32-12.bpo-45997.4n2aVU.rst @@ -0,0 +1 @@ +Fix :class:`asyncio.Semaphore` re-aquiring FIFO order. From 4e8f25eeac73d848c27de8cf8e1fb6d7ea1b6529 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 15 Mar 2022 18:58:21 +0200 Subject: [PATCH 07/10] Update Lib/asyncio/locks.py --- Lib/asyncio/locks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/asyncio/locks.py b/Lib/asyncio/locks.py index d35e4735b3886b..74a39c680e3f2e 100644 --- a/Lib/asyncio/locks.py +++ b/Lib/asyncio/locks.py @@ -386,7 +386,7 @@ async def acquire(self): self._wakeup_scheduled = False except exceptions.CancelledError: self._wakeup_scheduled = False - # The future was cancelled, wake up next waiter if any + # The future was cancelled, wake up the next waiter if any self._wake_up_next() raise self._value -= 1 From 6f193441d219eeaa66ad3e974d6d9809e7778999 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 15 Mar 2022 20:34:29 +0200 Subject: [PATCH 08/10] Simplify code --- Lib/asyncio/locks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/asyncio/locks.py b/Lib/asyncio/locks.py index 74a39c680e3f2e..bfb0a061e76576 100644 --- a/Lib/asyncio/locks.py +++ b/Lib/asyncio/locks.py @@ -385,7 +385,6 @@ async def acquire(self): # reset _wakeup_scheduled *after* waiting for a future self._wakeup_scheduled = False except exceptions.CancelledError: - self._wakeup_scheduled = False # The future was cancelled, wake up the next waiter if any self._wake_up_next() raise From 5078757829b05a9651b67d8cc8bb038a9057b149 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 15 Mar 2022 20:53:18 +0200 Subject: [PATCH 09/10] Drop redundand comment --- Lib/asyncio/locks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/asyncio/locks.py b/Lib/asyncio/locks.py index bfb0a061e76576..92b56309166068 100644 --- a/Lib/asyncio/locks.py +++ b/Lib/asyncio/locks.py @@ -385,7 +385,6 @@ async def acquire(self): # reset _wakeup_scheduled *after* waiting for a future self._wakeup_scheduled = False except exceptions.CancelledError: - # The future was cancelled, wake up the next waiter if any self._wake_up_next() raise self._value -= 1 From 5af4e19e9de529117056cdcfed09715e509221c4 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 15 Mar 2022 21:43:16 +0200 Subject: [PATCH 10/10] Add a comment --- Lib/asyncio/locks.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/asyncio/locks.py b/Lib/asyncio/locks.py index 92b56309166068..9b4612197de1d9 100644 --- a/Lib/asyncio/locks.py +++ b/Lib/asyncio/locks.py @@ -377,6 +377,8 @@ async def acquire(self): called release() to make it larger than 0, and then return True. """ + # _wakeup_scheduled is set if *another* task is scheduled to wakeup + # but its acquire() is not resumed yet while self._wakeup_scheduled or self._value <= 0: fut = self._get_loop().create_future() self._waiters.append(fut)