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

Tests sometimes fail with PyPy3 #140

Open
sbraz opened this issue Jan 8, 2022 · 9 comments
Open

Tests sometimes fail with PyPy3 #140

sbraz opened this issue Jan 8, 2022 · 9 comments

Comments

@sbraz
Copy link
Contributor

sbraz commented Jan 8, 2022

Hi,
It looks like tests sometimes fail when running PyPy3 (pypy-7.3.7-final):

test/test_plyvel.py::test_threading PASSEDpypy3: /build/leveldb-yUlUFD/leveldb-1.22/db/version_set.cc:779: leveldb::VersionSet::~VersionSet(): Assertion `dummy_versions_.next_ == &dummy_versions_' failed.
Fatal Python error: Aborted

Stack (most recent call first, approximate line numbers):
  File "/plyvel/test/test_plyvel.py", line 40 in finalize
  File "/opt/pypy/site-packages/_pytest/fixtures.py", line 1021 in finish
  File "/opt/pypy/lib_pypy/_functools.py", line 77 in __call__
  File "/opt/pypy/site-packages/_pytest/runner.py", line 389 in _callfinalizers
  File "/opt/pypy/site-packages/_pytest/runner.py", line 404 in _teardown_with_finalization
  File "/opt/pypy/site-packages/_pytest/runner.py", line 385 in _pop_and_teardown
  File "/opt/pypy/site-packages/_pytest/runner.py", line 421 in _teardown_towards
  File "/opt/pypy/site-packages/_pytest/runner.py", line 417 in teardown_exact
  File "/opt/pypy/site-packages/_pytest/runner.py", line 173 in pytest_runtest_teardown
  File "/opt/pypy/site-packages/pluggy/_callers.py", line 9 in _multicall
  File "/opt/pypy/site-packages/pluggy/_manager.py", line 77 in _hookexec
  File "/opt/pypy/site-packages/pluggy/_hooks.py", line 244 in __call__
  File "/opt/pypy/site-packages/_pytest/runner.py", line 255 in <lambda>
  File "/opt/pypy/site-packages/_pytest/runner.py", line 298 in from_call
  File "/opt/pypy/site-packages/_pytest/runner.py", line 240 in call_runtest_hook
  File "/opt/pypy/site-packages/_pytest/runner.py", line 212 in call_and_report
  File "/opt/pypy/site-packages/_pytest/runner.py", line 114 in runtestprotocol
  File "/opt/pypy/site-packages/_pytest/runner.py", line 106 in pytest_runtest_protocol
  File "/opt/pypy/site-packages/pluggy/_callers.py", line 9 in _multicall
  File "/opt/pypy/site-packages/pluggy/_manager.py", line 77 in _hookexec
  File "/opt/pypy/site-packages/pluggy/_hooks.py", line 244 in __call__
  File "/opt/pypy/site-packages/_pytest/main.py", line 336 in pytest_runtestloop
  File "/opt/pypy/site-packages/pluggy/_callers.py", line 9 in _multicall
  File "/opt/pypy/site-packages/pluggy/_manager.py", line 77 in _hookexec
  File "/opt/pypy/site-packages/pluggy/_hooks.py", line 244 in __call__
  File "/opt/pypy/site-packages/_pytest/main.py", line 319 in _main
  File "/opt/pypy/site-packages/_pytest/main.py", line 256 in wrap_session
  File "/opt/pypy/site-packages/_pytest/main.py", line 315 in pytest_cmdline_main
  File "/opt/pypy/site-packages/pluggy/_callers.py", line 9 in _multicall
  File "/opt/pypy/site-packages/pluggy/_manager.py", line 77 in _hookexec
  File "/opt/pypy/site-packages/pluggy/_hooks.py", line 244 in __call__
  File "/opt/pypy/site-packages/_pytest/config/__init__.py", line 130 in main
  File "/opt/pypy/site-packages/_pytest/config/__init__.py", line 178 in console_main
  File "/opt/pypy/bin/pytest", line 3 in <module>
  File "<builtin>/app_main.py", line 867 in execfile
  File "<builtin>/app_main.py", line 109 in run_toplevel
  File "<builtin>/app_main.py", line 607 in run_command_line
  File "<builtin>/app_main.py", line 944 in entry_point
Aborted (core dumped)

Other times, I get this non-fatal error:

======================= 52 passed, 1 deselected in 4.76s =======================
Exception ignored in: weakref callback <cyfunction __init__.<locals>.<lambda> at 0x7f1234686bd0>
Traceback (most recent call last):  
  File "plyvel/_plyvel.pyx", line 695, in plyvel._plyvel.BaseIterator.__init__.lambda
    lambda wr: ref_dict.pop(iterator_id))
KeyError: 139716944199424                  

Reproducer Dockerfile:

FROM pypy:3
RUN git clone --depth 1 https://github.com/wbolster/plyvel/
WORKDIR plyvel
RUN apt update
RUN apt install -y libleveldb-dev
RUN pip install cython pytest
RUN cython --cplus --fast-fail --annotate plyvel/_plyvel.pyx
RUN pypy3 setup.py install
RUN pytest --import-mode=append -vv --deselect test/test_plyvel.py::test_open_read_only_dir

Please note that this does not always fail, sometimes all tests pass. I believe the first error message has to do with threading since it happens immediately afterwards.

@wbolster
Copy link
Owner

wbolster commented Jan 8, 2022

interesting. this certainly looks like a threading issue and a race condition.

the second error is also in the threading test i assume?

@wbolster wbolster added the bug label Jan 8, 2022
@sbraz
Copy link
Contributor Author

sbraz commented Jan 9, 2022

I have really no idea what triggers the second error. I edited my first message to show that it happens after all tests have run. Strangely enough, it doesn't change pytest's exit code. Maybe it is a garbage collection issue?

@wbolster
Copy link
Owner

wbolster commented Jan 9, 2022

seems the latest cython release has some pypy3 incompatibilities, but i could build with a slightly older cython version.

running pytest, i also see this one, after all tests passed successfully:

test/test_plyvel.py::test_approximate_sizes
  /home/wbolster/Projects/wbolster/plyvel/.direnv/python-3.8.12/lib/pypy3.8/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: Exception ignored in: weakref callback : <cyfunction __init__.<locals>.<lambda> at 0x227aec0>
  
  Traceback (most recent call last):
    File "plyvel/_plyvel.pyx", line 695, in plyvel._plyvel.BaseIterator.__init__.lambda
      lambda wr: ref_dict.pop(iterator_id))
  KeyError: 140185035039968
  
    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))

@wbolster wbolster added the pypy label Jan 9, 2022
@wbolster
Copy link
Owner

wbolster commented Jan 9, 2022

ah, and with a few more retries i could reproduce the other crash as well.

note that pypy compatibility for python c extensions and cython is not ideal in general: https://cython.readthedocs.io/en/latest/src/userguide/pypy.html

wbolster added a commit that referenced this issue Jan 9, 2022
When running the tests on Pypy, sometimes this error occurs:

    Exception ignored in: weakref callback:
    <cyfunction __init__.<locals>.<lambda> at 0x227aec0>

      Traceback (most recent call last):
        File "plyvel/_plyvel.pyx", line 695, in plyvel._plyvel.BaseIterator.__init__.lambda
          lambda wr: ref_dict.pop(iterator_id))
      KeyError: 140185035039968

This issue manifests only sporadically, and may be related to cleanup
order (garbage collection etc.) being different on CPython and Pypy.

LevelDB requires that iterators are closed before a database is closed,
so plyvel tracks ‘active iterators’ using weak references (‘ref_dict’ in
the traceback above). Active iterators are cleaned up when a database is
closed. Apparently the weakref callback sometimes triggers after the
weakref has already been removed from the collection, and this raises
the KeyError. Ignoring this should be fine since it means the iterator
is closed already.

Simplify a bit by using a set instead of a dict to track the weakrefs,
and use set.discard() in the callback, which is a no-op (no exception)
for missing items.

See #140.
@wbolster
Copy link
Owner

wbolster commented Jan 9, 2022

i think i have a fix that should address the weakref callback error, and maybe the other issue as well. see #141 for details.

@sbraz
Copy link
Contributor Author

sbraz commented Jan 10, 2022

seems the latest cython release has some pypy3 incompatibilities, but i could build with a slightly older cython version.

Yes, I also ran into that so I rebuilt the extension with 0.29.24. For some reason I didn't have that issue with the PyPy3 container.

If you release a new version with the fix, can you please make sure the generated extension is compatible with

i think i have a fix that should address the weakref callback error, and maybe the other issue as well. see #141 for details.

I have run tests more than 100 times in a row and haven't any of the previous errors so I'd say this fix works :)
Out of curiosity does the KeyError have anything to do with the other error (leveldb-1.22/db/version_set.cc:779: leveldb::VersionSet::~VersionSet(): Assertion `dummy_versions_.next_ == &dummy_versions_' failed.)?

@wbolster
Copy link
Owner

i see cython has some recent pypy related activity: https://github.com/cython/cython/pulls?q=sort%3Aupdated-desc+pypy

and good question about the ‘other’ error. i really don't know :( but my results are consistent with yours. i can't reproduce, and i tried many times

@wbolster
Copy link
Owner

wbolster commented Jan 10, 2022

If you release a new version with the fix, can you please make sure the generated extension is compatible with

not sure i understand what you mean here?

@sbraz
Copy link
Contributor Author

sbraz commented Jan 10, 2022

The PyPI tarball for the latest version contains plyvel/_plyvel.cpp generated by Cython 0.29.26 which won't work with PyPy. It would be nice to have it generated by Cython 0.29.24.

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

No branches or pull requests

2 participants