From 381e50de53ec7c3577c57f44ab9ad252e43f1d42 Mon Sep 17 00:00:00 2001 From: Joongi Kim Date: Sun, 10 Oct 2021 18:17:13 +0900 Subject: [PATCH 1/8] bpo-45416: Fix use of asyncio.Condition() with explicit Lock objects --- Lib/asyncio/locks.py | 2 -- Lib/test/test_asyncio/test_locks.py | 37 ++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/Lib/asyncio/locks.py b/Lib/asyncio/locks.py index a7453fb1c77287..4fef64e3921e17 100644 --- a/Lib/asyncio/locks.py +++ b/Lib/asyncio/locks.py @@ -230,8 +230,6 @@ def __init__(self, lock=None, *, loop=mixins._marker): super().__init__(loop=loop) if lock is None: lock = Lock() - elif lock._loop is not self._get_loop(): - raise ValueError("loop argument must agree with lock") self._lock = lock # Export the lock's locked(), acquire() and release() methods. diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index 441adeea8f8e07..c5f3732da5ecb5 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -720,24 +720,39 @@ async def f(): self.loop.run_until_complete(f()) def test_explicit_lock(self): - lock = asyncio.Lock() - cond = asyncio.Condition(lock) + async def f(): + lock = asyncio.Lock() + cond = asyncio.Condition(lock) + self.assertIs(cond._lock, lock) + # should work same! + self.assertFalse(cond.locked()) + async with cond: + self.assertTrue(cond.locked()) + self.assertFalse(cond.locked()) - self.assertIs(cond._lock, lock) - self.assertIs(cond._loop, lock._loop) + self.loop.run_until_complete(f()) def test_ambiguous_loops(self): - loop = self.new_test_loop() - self.addCleanup(loop.close) - lock = asyncio.Lock() + loop = asyncio.new_event_loop() + self.addCleanup(loop.close) lock._loop = loop - async def _create_condition(): - with self.assertRaises(ValueError): - asyncio.Condition(lock) + async def f(): + async with lock: + # acquired immediately via the fast-path + # without interaction with any event loop. + cond = asyncio.Condition(lock) + # cond will trigger waiting on the lock + # and it will discover the event loop mismatch. + with self.assertRaisesRegex( + RuntimeError, + "is bound to a different event loop", + ): + async with cond: + pass - self.loop.run_until_complete(_create_condition()) + self.loop.run_until_complete(f()) def test_timeout_in_block(self): loop = asyncio.new_event_loop() From 1a6185ed993d589f0bf327f8aa33ece7ec92a262 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 10 Oct 2021 09:42:35 +0000 Subject: [PATCH 2/8] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2021-10-10-09-42-34.bpo-45416.n35O0_.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-10-10-09-42-34.bpo-45416.n35O0_.rst diff --git a/Misc/NEWS.d/next/Library/2021-10-10-09-42-34.bpo-45416.n35O0_.rst b/Misc/NEWS.d/next/Library/2021-10-10-09-42-34.bpo-45416.n35O0_.rst new file mode 100644 index 00000000000000..cf335d1bcd2c94 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-10-10-09-42-34.bpo-45416.n35O0_.rst @@ -0,0 +1,2 @@ +Fix use of :class:`asyncio.Condition` with explicit :class:`asyncio.Lock` objects, which was a regression due to removal of explicit loop arguments. +Patch by Joongi Kim. \ No newline at end of file From a156f1fe8540d8fa5cfb2a7db5d4879a3fefddae Mon Sep 17 00:00:00 2001 From: Joongi Kim Date: Sun, 10 Oct 2021 19:52:24 +0900 Subject: [PATCH 3/8] Apply review comments by @serhiy-storchaka --- Lib/test/test_asyncio/test_locks.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index c5f3732da5ecb5..f02ffbd54dfa1a 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -720,23 +720,33 @@ async def f(): self.loop.run_until_complete(f()) def test_explicit_lock(self): - async def f(): - lock = asyncio.Lock() - cond = asyncio.Condition(lock) + async def f(lock=None, cond=None): + if lock is None: + lock = asyncio.Lock() + if cond is None: + cond = asyncio.Condition(lock) self.assertIs(cond._lock, lock) # should work same! self.assertFalse(cond.locked()) async with cond: self.assertTrue(cond.locked()) self.assertFalse(cond.locked()) + async with lock: + self.assertTrue(lock.locked()) + self.assertFalse(lock.locked()) self.loop.run_until_complete(f()) + self.loop.run_until_complete(f(asyncio.Lock())) + lock = asyncio.Lock() + self.loop.run_until_complete(f(lock, asyncio.Condition(lock))) def test_ambiguous_loops(self): - lock = asyncio.Lock() loop = asyncio.new_event_loop() self.addCleanup(loop.close) - lock._loop = loop + with self.assertRaises(TypeError): + lock = asyncio.Lock(loop=loop) # actively disallowed since 3.10 + lock = asyncio.Lock() + lock._loop = loop # use private API for testing async def f(): async with lock: @@ -749,8 +759,7 @@ async def f(): RuntimeError, "is bound to a different event loop", ): - async with cond: - pass + await cond.acquire() self.loop.run_until_complete(f()) From ac0645da1094b487a5c0d73bfbe2e094898d89e5 Mon Sep 17 00:00:00 2001 From: Joongi Kim Date: Sun, 10 Oct 2021 19:55:08 +0900 Subject: [PATCH 4/8] Add more assertions to test_explicit_lock --- Lib/test/test_asyncio/test_locks.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index f02ffbd54dfa1a..2280d17074078f 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -726,10 +726,12 @@ async def f(lock=None, cond=None): if cond is None: cond = asyncio.Condition(lock) self.assertIs(cond._lock, lock) - # should work same! + self.assertFalse(lock.locked()) self.assertFalse(cond.locked()) async with cond: + self.assertTrue(lock.locked()) self.assertTrue(cond.locked()) + self.assertFalse(lock.locked()) self.assertFalse(cond.locked()) async with lock: self.assertTrue(lock.locked()) From 50c68c55660c3ee972b67400af95cc01f5021410 Mon Sep 17 00:00:00 2001 From: Joongi Kim Date: Sun, 10 Oct 2021 20:07:37 +0900 Subject: [PATCH 5/8] Add a test branch for cond.wait() --- Lib/test/test_asyncio/test_locks.py | 30 ++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index 2280d17074078f..33fd6b03774493 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -737,6 +737,7 @@ async def f(lock=None, cond=None): self.assertTrue(lock.locked()) self.assertFalse(lock.locked()) + # All should work in the same way. self.loop.run_until_complete(f()) self.loop.run_until_complete(f(asyncio.Lock())) lock = asyncio.Lock() @@ -745,17 +746,17 @@ async def f(lock=None, cond=None): def test_ambiguous_loops(self): loop = asyncio.new_event_loop() self.addCleanup(loop.close) - with self.assertRaises(TypeError): - lock = asyncio.Lock(loop=loop) # actively disallowed since 3.10 - lock = asyncio.Lock() - lock._loop = loop # use private API for testing - async def f(): + async def wrong_loop_in_lock(): + with self.assertRaises(TypeError): + lock = asyncio.Lock(loop=loop) # actively disallowed since 3.10 + lock = asyncio.Lock() + lock._loop = loop # use private API for testing async with lock: # acquired immediately via the fast-path # without interaction with any event loop. cond = asyncio.Condition(lock) - # cond will trigger waiting on the lock + # cond.acquire() will trigger waiting on the lock # and it will discover the event loop mismatch. with self.assertRaisesRegex( RuntimeError, @@ -763,7 +764,22 @@ async def f(): ): await cond.acquire() - self.loop.run_until_complete(f()) + async def wrong_loop_in_cond(): + # Same analogy here with the condition's loop. + lock = asyncio.Lock() + async with lock: + cond = asyncio.Condition(lock) + with self.assertRaises(TypeError): + cond = asyncio.Condition(lock, loop=loop) + cond._loop = loop + with self.assertRaisesRegex( + RuntimeError, + "is bound to a different event loop", + ): + await cond.wait() + + self.loop.run_until_complete(wrong_loop_in_lock()) + self.loop.run_until_complete(wrong_loop_in_cond()) def test_timeout_in_block(self): loop = asyncio.new_event_loop() From fae4b91f6ffea94f0f3026d4055a6b4df95a4401 Mon Sep 17 00:00:00 2001 From: Joongi Kim Date: Mon, 11 Oct 2021 00:20:17 +0900 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Serhiy Storchaka --- Lib/test/test_asyncio/test_locks.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index 33fd6b03774493..c31cecc8682db5 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -735,7 +735,9 @@ async def f(lock=None, cond=None): self.assertFalse(cond.locked()) async with lock: self.assertTrue(lock.locked()) + self.assertTrue(cond.locked()) self.assertFalse(lock.locked()) + self.assertFalse(cond.locked()) # All should work in the same way. self.loop.run_until_complete(f()) @@ -749,7 +751,7 @@ def test_ambiguous_loops(self): async def wrong_loop_in_lock(): with self.assertRaises(TypeError): - lock = asyncio.Lock(loop=loop) # actively disallowed since 3.10 + asyncio.Lock(loop=loop) # actively disallowed since 3.10 lock = asyncio.Lock() lock._loop = loop # use private API for testing async with lock: From 615ce7b31c70cc9ec31a9a7bf23b45713028a685 Mon Sep 17 00:00:00 2001 From: Joongi Kim Date: Mon, 11 Oct 2021 00:33:11 +0900 Subject: [PATCH 7/8] Fix another same pattern as noted in the prior review --- Lib/test/test_asyncio/test_locks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index c31cecc8682db5..00e6ea020d0be9 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -772,7 +772,7 @@ async def wrong_loop_in_cond(): async with lock: cond = asyncio.Condition(lock) with self.assertRaises(TypeError): - cond = asyncio.Condition(lock, loop=loop) + asyncio.Condition(lock, loop=loop) cond._loop = loop with self.assertRaisesRegex( RuntimeError, From b62d6752803b1a4eff483125fcfeac97465f991b Mon Sep 17 00:00:00 2001 From: Joongi Kim Date: Mon, 11 Oct 2021 00:34:57 +0900 Subject: [PATCH 8/8] Make it more readable --- Lib/test/test_asyncio/test_locks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_locks.py b/Lib/test/test_asyncio/test_locks.py index 00e6ea020d0be9..b2492c1acfecef 100644 --- a/Lib/test/test_asyncio/test_locks.py +++ b/Lib/test/test_asyncio/test_locks.py @@ -770,9 +770,9 @@ async def wrong_loop_in_cond(): # Same analogy here with the condition's loop. lock = asyncio.Lock() async with lock: - cond = asyncio.Condition(lock) with self.assertRaises(TypeError): asyncio.Condition(lock, loop=loop) + cond = asyncio.Condition(lock) cond._loop = loop with self.assertRaisesRegex( RuntimeError,