Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
37 changes: 26 additions & 11 deletions Lib/test/test_asyncio/test_locks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to set it using only public API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with it, but it says:

TypeError: As of 3.10, the *loop* parameter was removed from Lock() since it is no longer necessary

So I've added an explicit test for the TypeError and resorted back to lock._loop = loop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant using the public API which implicitly sets _loop. But it would be not easy. We can leave it to following PRs.


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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exception is raised in await cond.acquire(). I think it would be better to call it directly.

Suggested change
async with cond:
pass
await cond.acquire()

It is raised because cond.acquire is the same as lock.acquire, and the latter fails because its loop is different.

But it would be nice to test less trivial case: cond.wait() which should raise RuntimeError in different code.


self.loop.run_until_complete(_create_condition())
self.loop.run_until_complete(f())

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.