Merge branch 'master' of github.com:zopefoundation/zope.interface into master

5 files changed, 132 insertions, 7 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 84c8389..a988b41 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -2,7 +2,16 @@
  Changes
 =========
 
-5.1.1 (unreleased)
+5.1.2 (unreleased)
+==================
+
+- Make sure to call each invariant only once when validating invariants.
+  Previously, invariants could be called multiple times because when an
+  invariant is defined in an interface, it's found by in all interfaces
+  inheriting from that interface.  See `pull request 215
+  <https://github.com/zopefoundation/zope.interface/pull/215/>`_.
+
+5.1.1 (2020-09-30)
 ==================
 
 - Fix the method definitions of ``IAdapterRegistry.subscribe``,
@@ -12,11 +21,32 @@
   that argument. See `issue 208
   <https://github.com/zopefoundation/zope.interface/issues/208>`_.
 
-- Make sure to call each invariant only once when validating invariants.
-  Previously, invariants could be called multiple times because when an
-  invariant is defined in an interface, it's found by in all interfaces
-  inheriting from that interface.  See `pull request 215
-  <https://github.com/zopefoundation/zope.interface/pull/215/>`_.
+- Fix a potential reference leak in the C optimizations. Previously,
+  applications that dynamically created unique ``Specification``
+  objects (e.g., used ``@implementer`` on dynamic classes) could
+  notice a growth of small objects over time leading to increased
+  garbage collection times. See `issue 216
+  <https://github.com/zopefoundation/zope.interface/issues/216>`_.
+
+  .. caution::
+
+     This leak could prevent interfaces used as the bases of
+     other interfaces from being garbage collected. Those interfaces
+     will now be collected.
+
+     One way in which this would manifest was that ``weakref.ref``
+     objects (and things built upon them, like
+     ``Weak[Key|Value]Dictionary``) would continue to have access to
+     the original object even if there were no other visible
+     references to Python and the original object *should* have been
+     collected. This could be especially problematic for the
+     ``WeakKeyDictionary`` when combined with dynamic or local
+     (created in the scope of a function) interfaces, since interfaces
+     are hashed based just on their name and module name. See the
+     linked issue for an example of a resulting ``KeyError``.
+
+     Note that such potential errors are not new, they are just once
+     again a possibility.
 
 5.1.0 (2020-04-08)
 ==================
diff --git a/docs/api/specifications.rst b/docs/api/specifications.rst
index 357e361..45cf5e1 100644
--- a/docs/api/specifications.rst
+++ b/docs/api/specifications.rst
@@ -161,6 +161,70 @@ Exmples for :meth:`.Specification.extends`:
    >>> I2.extends(I2, strict=False)
    True
 
+Equality, Hashing, and Comparisons
+----------------------------------
+
+Specifications (including their notable subclass `Interface`), are
+hashed and compared based solely on their ``__name__`` and
+``__module__``, not including any information about their enclosing
+scope, if any (e.g., their ``__qualname__``). This means that any two
+objects created with the same name and module are considered equal and
+map to the same value in a dictionary.
+
+.. doctest::
+
+   >>> from zope.interface import Interface
+   >>> class I1(Interface): pass
+   >>> orig_I1 = I1
+   >>> class I1(Interface): pass
+   >>> I1 is orig_I1
+   False
+   >>> I1 == orig_I1
+   True
+   >>> d = {I1: 42}
+   >>> d[orig_I1]
+   42
+   >>> def make_nested():
+   ...     class I1(Interface): pass
+   ...     return I1
+   >>> nested_I1 = make_nested()
+   >>> I1 == orig_I1 == nested_I1
+   True
+
+Because weak references hash the same as their underlying object,
+this can lead to surprising results when weak references are involved,
+especially if there are cycles involved or if the garbage collector is
+not based on reference counting (e.g., PyPy). For example, if you
+redefine an interface named the same as an interface being used in a
+``WeakKeyDictionary``, you can get a ``KeyError``, even if you put the
+new interface into the dictionary.
+
+
+.. doctest::
+
+   >>> from zope.interface import Interface
+   >>> import gc
+   >>> from weakref import WeakKeyDictionary
+   >>> wr_dict = WeakKeyDictionary()
+   >>> class I1(Interface): pass
+   >>> wr_dict[I1] = 42
+   >>> orig_I1 = I1 # Make sure it stays alive
+   >>> class I1(Interface): pass
+   >>> wr_dict[I1] = 2020
+   >>> del orig_I1
+   >>> _ = gc.collect() # Sometime later, gc runs and makes sure the original is gone
+   >>> wr_dict[I1] # Cleaning up the original weakref removed the new one
+   Traceback (most recent call last):
+   ...
+   KeyError: ...
+
+This is mostly likely a problem in test cases where it is tempting to
+use the same named interfaces in different test methods. If references
+to them escape, especially if they are used as the bases of other
+interfaces, you may find surprising ``KeyError`` exceptions. For this
+reason, it is best to use distinct names for local interfaces within
+the same test module.
+
 
 Interface
 =========
diff --git a/setup.py b/setup.py
index fb32743..ba4c7d7 100644
--- a/setup.py
+++ b/setup.py
@@ -100,7 +100,7 @@ long_description = (
         )
 
 setup(name='zope.interface',
-      version='5.1.1.dev0',
+      version='5.1.2.dev0',
       url='https://github.com/zopefoundation/zope.interface',
       license='ZPL 2.1',
       description='Interfaces for Python',
diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c
index 346e7f2..374311e 100644
--- a/src/zope/interface/_zope_interface_coptimizations.c
+++ b/src/zope/interface/_zope_interface_coptimizations.c
@@ -350,6 +350,7 @@ Spec_traverse(Spec* self, visitproc visit, void* arg)
 {
     Py_VISIT(self->_implied);
     Py_VISIT(self->_dependents);
+    Py_VISIT(self->_bases);
     Py_VISIT(self->_v_attrs);
     Py_VISIT(self->__iro__);
     Py_VISIT(self->__sro__);
@@ -361,6 +362,7 @@ Spec_clear(Spec* self)
 {
     Py_CLEAR(self->_implied);
     Py_CLEAR(self->_dependents);
+    Py_CLEAR(self->_bases);
     Py_CLEAR(self->_v_attrs);
     Py_CLEAR(self->__iro__);
     Py_CLEAR(self->__sro__);
diff --git a/src/zope/interface/tests/test_declarations.py b/src/zope/interface/tests/test_declarations.py
index c8678b5..9ef192a 100644
--- a/src/zope/interface/tests/test_declarations.py
+++ b/src/zope/interface/tests/test_declarations.py
@@ -1078,6 +1078,35 @@ class Test_implementer(Test_classImplements):
         self.assertIsNone(spec.inherit,)
         self.assertIs(foo.__implemented__, spec) # pylint:disable=no-member
 
+    def test_does_not_leak_on_unique_classes(self):
+        # Make sure nothing is hanging on to the class or Implements
+        # object after they go out of scope. There was briefly a bug
+        # in 5.x that caused SpecificationBase._bases (in C) to not be
+        # traversed or cleared.
+        # https://github.com/zopefoundation/zope.interface/issues/216
+        import gc
+        from zope.interface.interface import InterfaceClass
+        IFoo = InterfaceClass('IFoo')
+
+        begin_count = len(gc.get_objects())
+
+        for _ in range(1900):
+            class TestClass(object):
+                pass
+
+            self._callFUT(TestClass, IFoo)
+
+        gc.collect()
+
+        end_count = len(gc.get_objects())
+
+        # How many new objects might still be around? In all currently
+        # tested interpreters, there aren't any, so our counts should
+        # match exactly. When the bug existed, in a steady state, the loop
+        # would grow by two objects each iteration
+        fudge_factor = 0
+        self.assertLessEqual(end_count, begin_count + fudge_factor)
+
 
 
 class Test_implementer_only(Test_classImplementsOnly):