Skip to content

bpo-45416: Fix use of asyncio.Condition() with explicit Lock objects #28850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Lib/asyncio/locks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
70 changes: 57 additions & 13 deletions Lib/test/test_asyncio/test_locks.py
Original file line number Diff line number Diff line change
Expand Up @@ -720,24 +720,68 @@ 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=None, cond=None):
if lock is None:
lock = asyncio.Lock()
if cond is None:
cond = asyncio.Condition(lock)
self.assertIs(cond._lock, lock)
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())
self.assertTrue(cond.locked())
self.assertFalse(lock.locked())
self.assertFalse(cond.locked())

self.assertIs(cond._lock, lock)
self.assertIs(cond._loop, lock._loop)
# All should work in the same way.
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):
loop = self.new_test_loop()
loop = asyncio.new_event_loop()
self.addCleanup(loop.close)

lock = asyncio.Lock()
lock._loop = loop

async def _create_condition():
with self.assertRaises(ValueError):
asyncio.Condition(lock)

self.loop.run_until_complete(_create_condition())
async def wrong_loop_in_lock():
with self.assertRaises(TypeError):
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.acquire() 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",
):
await cond.acquire()

async def wrong_loop_in_cond():
# Same analogy here with the condition's loop.
lock = asyncio.Lock()
async with lock:
with self.assertRaises(TypeError):
asyncio.Condition(lock, loop=loop)
cond = asyncio.Condition(lock)
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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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.