Skip to content
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

Error with Python 3.10: "ValueError: loop argument must agree with lock" #358

Closed
simonw opened this issue Oct 8, 2021 · 10 comments
Closed

Error with Python 3.10: "ValueError: loop argument must agree with lock" #358

simonw opened this issue Oct 8, 2021 · 10 comments

Comments

@simonw
Copy link
Contributor

@simonw simonw commented Oct 8, 2021

I ran into this in my own project, see simonw/datasette#1481 - then I tried using a fork of this project to run the unit tests against Python 3.10 and got the same error: https://github.com/simonw/janus/runs/3842463703?check_suite_focus=true

 ============================= test session starts ==============================
 platform linux -- Python 3.10.0, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
 rootdir: /home/runner/work/janus/janus
 plugins: cov-2.12.1, asyncio-0.15.1
 collected 72 items
 
 tests/test_async.py FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF                      [ 43%]
 tests/test_mixed.py .FFFFFFFFFFFFFFFFF                                   [ 68%]
 tests/test_sync.py FFFFFFFFFFFFFFFFFFFFFFF                               [100%]
 
 =================================== FAILURES ===================================
 __________________________ TestQueueBasic.test_empty ___________________________
 
 self = <test_async.TestQueueBasic object at 0x7fb0f561e4d0>
 
     @pytest.mark.asyncio
     async def test_empty(self):
 >       _q = janus.Queue()
 
 tests/test_async.py:65: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 janus/__init__.py:39: in __init__
     self._async_not_empty = asyncio.Condition(self._async_mutex)
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 self = <[AttributeError("'Condition' object has no attribute 'locked'") raised in repr()] Condition object at 0x7fb0f569a620>
 lock = <asyncio.locks.Lock object at 0x7fb0f569ada0 [unlocked]>
 
     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")
 E           ValueError: loop argument must agree with lock
@simonw
Copy link
Contributor Author

@simonw simonw commented Oct 8, 2021

The code involved here is the Queue constructor:

janus/janus/__init__.py

Lines 24 to 42 in d7970f8

class Queue(Generic[T]):
def __init__(self, maxsize: int = 0) -> None:
self._loop = current_loop()
self._maxsize = maxsize
self._init(maxsize)
self._unfinished_tasks = 0
self._sync_mutex = threading.Lock()
self._sync_not_empty = threading.Condition(self._sync_mutex)
self._sync_not_full = threading.Condition(self._sync_mutex)
self._all_tasks_done = threading.Condition(self._sync_mutex)
self._async_mutex = asyncio.Lock()
self._async_not_empty = asyncio.Condition(self._async_mutex)
self._async_not_full = asyncio.Condition(self._async_mutex)
self._finished = asyncio.Event()
self._finished.set()

@simonw
Copy link
Contributor Author

@simonw simonw commented Oct 8, 2021

In particular it seems to be these two lines:

janus/janus/__init__.py

Lines 38 to 39 in d7970f8

self._async_mutex = asyncio.Lock()
self._async_not_empty = asyncio.Condition(self._async_mutex)

I don't understand how those are any different from doing this: asyncio.Condition(asyncio.Lock()) - which works fine when I try it in the Python 3.10 interpreter:

~ % /Users/simon/.pyenv/versions/3.10.0/bin/python
Python 3.10.0 (default, Oct  7 2021, 13:45:58) [Clang 12.0.0 (clang-1200.0.32.29)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> asyncio.Condition(asyncio.Lock())
<asyncio.locks.Condition object at 0x10ace2110 [unlocked]>

@simonw
Copy link
Contributor Author

@simonw simonw commented Oct 8, 2021

Aha! But that simple asyncio.Condition(asyncio.Lock()) line DOES fail if it runs inside an event loop:

>>> async def run():
...     print(asyncio.Condition(asyncio.Lock()))
... 
>>> asyncio.run(run())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/simon/.pyenv/versions/3.10.0/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/Users/simon/.pyenv/versions/3.10.0/lib/python3.10/asyncio/base_events.py", line 641, in run_until_complete
    return future.result()
  File "<stdin>", line 2, in run
  File "/Users/simon/.pyenv/versions/3.10.0/lib/python3.10/asyncio/locks.py", line 234, in __init__
    raise ValueError("loop argument must agree with lock")
ValueError: loop argument must agree with lock

@simonw
Copy link
Contributor Author

@simonw simonw commented Oct 8, 2021

Could this be a bug in Python 3.10?

@simonw
Copy link
Contributor Author

@simonw simonw commented Oct 8, 2021

I filed a Python 3.10 issue about this here: https://bugs.python.org/issue45416

@ambv
Copy link

@ambv ambv commented Oct 8, 2021

I commented on BPO-45416 with a workaround that janus can employ.

@simonw
Copy link
Contributor Author

@simonw simonw commented Oct 8, 2021

Here's the first of @ambv's suggested workaround (there's a longer but less hacky one in the comment too):

>>> l = asyncio.Lock()
>>> getattr(l, '_get_loop', lambda: None)()

I'm going to try that against Janus now.

@simonw
Copy link
Contributor Author

@simonw simonw commented Oct 8, 2021

That addressed the bug!

diff --git a/janus/__init__.py b/janus/__init__.py
index 789f12f..a2375b6 100644
--- a/janus/__init__.py
+++ b/janus/__init__.py
@@ -36,6 +36,8 @@ class Queue(Generic[T]):
         self._all_tasks_done = threading.Condition(self._sync_mutex)
 
         self._async_mutex = asyncio.Lock()
+        # Workaround for issue #358:
+        getattr(self._async_mutex, '_get_loop', lambda: None)()
         self._async_not_empty = asyncio.Condition(self._async_mutex)
         self._async_not_full = asyncio.Condition(self._async_mutex)
         self._finished = asyncio.Event()
(janus) janus % pytest -x      
============================================================= test session starts ==============================================================
platform darwin -- Python 3.10.0, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /Users/simon/Dropbox/Development/janus
plugins: cov-2.12.1, asyncio-0.15.1
collected 72 items                                                                                                                             

tests/test_async.py ...............................                                                                                      [ 43%]
tests/test_mixed.py ..................                                                                                                   [ 68%]
tests/test_sync.py .......................                                                                                               [100%]

============================================================== 72 passed in 5.34s ==============================================================

@achimnol
Copy link
Member

@achimnol achimnol commented Oct 10, 2021

I have sent a PR to CPython: python/cpython#28850.

asvetlov added a commit that referenced this issue Oct 24, 2021
Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
@asvetlov asvetlov closed this Oct 24, 2021
@asvetlov
Copy link
Member

@asvetlov asvetlov commented Oct 24, 2021

janus 0.6.2 released with the fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants