Skip to content

Commit

Permalink
Fix the CFFI cache implementation to not print AttributeErrors.
Browse files Browse the repository at this point in the history
Only affects CPython when PURE_PYTHON=1.

Add tests for this. They're a bit dicey because this is a hard situation to get into, but they do test the symptoms.

Fixes #150
  • Loading branch information
jamadden committed Mar 10, 2021
1 parent 6b500fc commit 3a17b12
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGES.rst
Expand Up @@ -12,6 +12,10 @@
when setting its ``__class__`` and ``__dict__``. This matches the
behaviour of the C implementation. See `issue 155
<https://github.com/zopefoundation/persistent/issues/155>`_.
- Fix the CFFI cache implementation (used on CPython when
``PURE_PYTHON=1``) to not print unraisable ``AttributeErrors`` from
``_WeakValueDictionary`` during garbage collection. See `issue 150
<https://github.com/zopefoundation/persistent/issues/150>`_.

4.6.4 (2020-03-26)
==================
Expand Down
8 changes: 7 additions & 1 deletion persistent/picklecache.py
Expand Up @@ -102,7 +102,13 @@ def _from_addr(self, addr):
return self._cast(addr, self._py_object).value

def cleanup_hook(self, cdata):
oid = self._addr_to_oid.pop(cdata.pobj_id, None)
# This is called during GC, possibly at interpreter shutdown
# when the __dict__ of this object may have already been cleared.
try:
addr_to_oid = self._addr_to_oid
except AttributeError:
return
oid = addr_to_oid.pop(cdata.pobj_id, None)
self._data.pop(oid, None)

def __contains__(self, oid):
Expand Down
63 changes: 63 additions & 0 deletions persistent/tests/test_picklecache.py
Expand Up @@ -1102,6 +1102,45 @@ def _p_deactivate(self):
self.assertEqual(cache.cache_non_ghost_count, 0)
self.assertEqual(len(cache), 0)

def test_interpreter_finalization_ffi_cleanup(self):
# When the interpreter is busy garbage collecting old objects
# and clearing their __dict__ in random orders, the CFFI cleanup
# ``ffi.gc()`` cleanup hooks we use on CPython don't
# raise errors.
#
# Prior to Python 3.8, when ``sys.unraisablehook`` was added,
# the only way to know if this test fails is to look for AttributeError
# on stderr.
#
# But wait, it gets worse. Prior to https://foss.heptapod.net/pypy/cffi/-/issues/492
# (CFFI > 1.14.5, unreleased at this writing), CFFI ignores
# ``sys.unraisablehook``, so even on 3.8 the only way to know
# a failure is to watch stderr.
#
# See https://github.com/zopefoundation/persistent/issues/150

import sys
unraised = []
try:
old_hook = sys.unraisablehook
except AttributeError:
pass
else:
def new_hook(unraisable):
unraised.append(1)
sys.unraisablehook = new_hook
self.addCleanup(setattr, sys, 'unraisablehook', old_hook)

cache = self._makeOne()
oid = self._numbered_oid(42)
o = cache[oid] = self._makePersist(oid=oid)
# Clear the dict, or at least part of it.
# This is coupled to ``cleanup_hook``
del cache.data._addr_to_oid
del cache[oid]

self.assertEqual(unraised, [])


@skipIfNoCExtension
class CPickleCacheTests(PickleCacheTestMixin, unittest.TestCase):
Expand Down Expand Up @@ -1182,6 +1221,30 @@ def makePersistent(oid):
self.assertEqual(len(cache), 0)


class TestWeakValueDictionary(unittest.TestCase):

def _getTargetClass(self):
from persistent.picklecache import _WeakValueDictionary
return _WeakValueDictionary

def _makeOne(self):
return self._getTargetClass()()

@unittest.skipIf(PYPY, "PyPy doesn't have the cleanup_hook")
def test_cleanup_hook_gc(self):
# A more targeted test than ``test_interpreter_finalization_ffi_cleanup``
# See https://github.com/zopefoundation/persistent/issues/150
wvd = self._makeOne()

class cdata(object):
o = object()
pobj_id = id(o)
wvd['key'] = cdata.o

wvd.__dict__.clear()
wvd.cleanup_hook(cdata)


def test_suite():
return unittest.defaultTestLoader.loadTestsFromName(__name__)

Expand Down

0 comments on commit 3a17b12

Please sign in to comment.