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

Allow weakref callback to be a no-op during iterator cleanup #141

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wbolster
Copy link
Owner

@wbolster wbolster commented 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.

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 wbolster self-assigned this Jan 9, 2022
@wbolster
Copy link
Owner Author

wbolster commented Jan 9, 2022

@sbraz any chance you can give this branch a spin?

i tried many times and don't see any errors anymore. the other error in #140 is very puzzling, and perhaps it's also fixed by this PR, but i'm not sure

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

Successfully merging this pull request may close these issues.

1 participant