Partially revert #84 in order to fix #85issue85

But add testing to be sure it doesn't break again, and extra
suspenders to ensure the issues that #84 was trying to deal with
doesn't show up either.

5 files changed, 104 insertions, 22 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 92f3762..f94f3c2 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,15 +4,17 @@ Changes
 4.4.2 (unreleased)
 ------------------
 
-- Nothing changed yet.
-
+- Fix a regression storing
+  ``zope.component.persistentregistry.PersistentRegistry`` instances.
+  See `issue 85 <https://github.com/zopefoundation/zope.interface/issues/85>`_.
 
 4.4.1 (2017-05-13)
 ------------------
 
 - Simplify the caching of utility-registration data. In addition to
   simplification, avoids spurious test failures when checking for
-  leaks in tests with persistent registries.
+  leaks in tests with persistent registries. See `pull 84
+  <https://github.com/zopefoundation/zope.interface/pull/84>`_.
 
 - Raise ``ValueError`` when non-text names are passed to adapter registry
   methods:  prevents corruption of lookup caches.
diff --git a/setup.py b/setup.py
index 1b6fb13..630a11a 100644
--- a/setup.py
+++ b/setup.py
@@ -82,7 +82,10 @@ if is_pypy or is_jython or is_pure:
     features = {}
 else:
     features = {'codeoptimization': codeoptimization}
-tests_require = ['zope.event']
+tests_require = [
+    'zope.event',
+    'zope.testing',
+]
 testing_extras = tests_require + ['nose', 'coverage']
 
 
diff --git a/src/zope/interface/registry.py b/src/zope/interface/registry.py
index 8539d27..7c853ce 100644
--- a/src/zope/interface/registry.py
+++ b/src/zope/interface/registry.py
@@ -14,6 +14,8 @@
 """Basic components support
 """
 from collections import defaultdict
+import functools
+import weakref
 
 try:
     from zope.event import notify
@@ -70,6 +72,53 @@ class _UnhashableComponentCounter(object):
 
 class _UtilityRegistrations(object):
 
+    # Strong reference {id(components): _UtilityRegistrations}
+    _regs_for_components = {}
+    # Weak reference {id(components): components}, used for cleanup.
+    _weakrefs_for_components = {}
+
+    @classmethod
+    def for_components(cls, components):
+        # We manage these utility/subscription registrations as associated
+        # objects with a weakref to avoid making any changes to
+        # the pickle format. They are keyed off the id of the component because
+        # Components subclasses are not guaranteed to be hashable.
+        key = id(components)
+        try:
+            regs = cls._regs_for_components[key]
+        except KeyError:
+            regs = None
+        else:
+            # In case the components have been re-initted, clear the cache
+            # (zope.component.testing does this between tests, which calls Components.__init__,
+            # so we should typically not get here)
+            if (regs._utilities is not components.utilities
+                or regs._utility_registrations is not components._utility_registrations):
+                regs = None # pragma: no cover
+
+        if regs is None:
+            regs = cls(components.utilities, components._utility_registrations)
+            cls._regs_for_components[key] = regs
+
+            if key not in cls._weakrefs_for_components:
+                cleanup = functools.partial(cls._cleanup_for_components, key)
+                cls._weakrefs_for_components[key] = weakref.ref(components, cleanup)
+
+        return regs
+
+    @classmethod
+    def _cleanup_for_components(cls, key, *args):
+        cls._weakrefs_for_components.pop(key, None)
+        cls._regs_for_components.pop(key, None)
+
+    @classmethod
+    def reset_for_components(cls, components):
+        cls._cleanup_for_components(id(components))
+
+    @classmethod
+    def clear_cache(cls):
+        cls._regs_for_components.clear()
+
     def __init__(self, utilities, utility_registrations):
         # {provided -> {component: count}}
         self._cache = defaultdict(lambda: defaultdict(int))
@@ -134,6 +183,12 @@ class _UtilityRegistrations(object):
         if not subscribed:
             self._utilities.unsubscribe((), provided, component)
 
+try:
+    from zope.testing import cleanup
+except ImportError: # pragma: no cover
+    pass
+else:
+    cleanup.addCleanUp(_UtilityRegistrations.clear_cache)
 
 @implementer(IComponents)
 class Components(object):
@@ -147,14 +202,14 @@ class Components(object):
 
         # __init__ is used for test cleanup as well as initialization.
         # XXX add a separate API for test cleanup.
-        # See _utility_registrations below.
-        if hasattr(self, '_v_utility_registrations_cache'):
-            del self._v_utility_registrations_cache
+        _UtilityRegistrations.reset_for_components(self)
 
     def __repr__(self):
         return "<%s %s>" % (self.__class__.__name__, self.__name__)
 
     def _init_registries(self):
+        # Subclasses have never been required to call this, merely to implement
+        # it to initialize these two properties.
         self.adapters = AdapterRegistry()
         self.utilities = AdapterRegistry()
 
@@ -166,16 +221,12 @@ class Components(object):
 
     @property
     def _utility_registrations_cache(self):
-        # We use a _v_ attribute internally so that data aren't saved in ZODB.
-        # If data are pickled in other contexts, the data will be carried along.
-        # There's no harm in pickling the extra data othr than that it would
-        # be somewhat wasteful. It's doubtful that that's an issue anyway.
-        try:
-            return self._v_utility_registrations_cache
-        except AttributeError:
-            self._v_utility_registrations_cache = _UtilityRegistrations(
-                self.utilities, self._utility_registrations)
-            return self._v_utility_registrations_cache
+        # We go through the class mapping to be sure that this never
+        # gets pickled. There are "persistent" subclasses of us that aren't
+        # actually Persistent objects themselves (only their registries are)
+        # so using a _v name won't work. Similarly, there are subclasses
+        # that inherit from dict too so overriding __getstate__ won't work.
+        return _UtilityRegistrations.for_components(self)
 
     def _getBases(self):
         # Subclasses might override
@@ -192,7 +243,7 @@ class Components(object):
     __bases__ = property(
         lambda self: self._getBases(),
         lambda self, bases: self._setBases(bases),
-        )
+    )
 
     def registerUtility(self, component=None, provided=None, name=u'',
                         info=u'', event=True, factory=None):
diff --git a/src/zope/interface/tests/test_registry.py b/src/zope/interface/tests/test_registry.py
index 3e71aff..00aea76 100644
--- a/src/zope/interface/tests/test_registry.py
+++ b/src/zope/interface/tests/test_registry.py
@@ -577,7 +577,12 @@ class ComponentsTests(unittest.TestCase):
 
         # zope.component.testing does this
         comp.__init__('base')
+
+        # And let's go ahead and destroy the cache at random times too
+        from zope.interface.registry import _UtilityRegistrations
+        _UtilityRegistrations.clear_cache()
         comp.registerUtility(_to_reg, ifoo, _name2, _info)
+        _UtilityRegistrations.clear_cache()
 
         _monkey, _events = self._wrapEvents()
         with _monkey:
@@ -2645,12 +2650,21 @@ class PersistentComponents(Components):
         self.adapters = PersistentAdapterRegistry()
         self.utilities = PersistentAdapterRegistry()
 
+class PersistentDictComponents(PersistentComponents, dict):
+    # Like Pyramid's Registry, we subclass Components and dict
+    pass
 
 class TestPersistentComponents(unittest.TestCase):
 
+    def _makeOne(self):
+        return PersistentComponents('test')
+
+    def _check_equality_after_pickle(self, made):
+        pass
+
     def test_pickles_empty(self):
         import pickle
-        comp = PersistentComponents('test')
+        comp = self._makeOne()
         pickle.dumps(comp)
         comp2 = pickle.loads(pickle.dumps(comp))
 
@@ -2658,7 +2672,7 @@ class TestPersistentComponents(unittest.TestCase):
 
     def test_pickles_with_utility_registration(self):
         import pickle
-        comp = PersistentComponents('test')
+        comp = self._makeOne()
         comp.registerUtility(
             object(),
             Interface)
@@ -2666,7 +2680,19 @@ class TestPersistentComponents(unittest.TestCase):
         comp2 = pickle.loads(pickle.dumps(comp))
         self.assertEqual(comp2.__name__, 'test')
 
-        self.assertNotNone(comp2.getUtility(Interface))
+        self.assertIsNotNone(comp2.getUtility(Interface))
+        self._check_equality_after_pickle(comp2)
+
+class TestPersistentDictComponents(TestPersistentComponents):
+
+    def _makeOne(self):
+        comp = PersistentDictComponents('test')
+        comp['key'] = 42
+        return comp
+
+    def _check_equality_after_pickle(self, made):
+        self.assertIn('key', made)
+        self.assertEqual(made['key'], 42)
 
 
 class _Monkey(object):
diff --git a/tox.ini b/tox.ini
index 608fa00..28e2df3 100644
--- a/tox.ini
+++ b/tox.ini
@@ -6,7 +6,7 @@ envlist =
 commands =
     python setup.py -q test -q {posargs}
 deps =
-    zope.event
+    .[test]
 
 [testenv:py27-pure]
 setenv =