Merge pull request #106 from NextThought/issue104

Rely on zope.interface 5.0, and work around zope.container/issues/38

.travis.yml

@@ -2,26 +2,29 @@ language: python
 group: travis_latest
 language: python
 python:
-- 2.7
-- 3.6
-- 3.7
-- 3.8
-- pypy
-- pypy3
+  - 2.7
+  - 3.6
+  - 3.7
+  - 3.8
+  - pypy
+  - pypy3
 jobs:
     include:
       - python: 2.7
         env: PURE_PYTHON=1
       - python: 3.8
         env: PURE_PYTHON=1
 env:
-    global:
-        - CC="ccache gcc"
-        - CCACHE_SLOPPINESS=file_macro,time_macros,include_file_ctime,include_file_mtime
-        - CCACHE_NOHASHDIR=true
-        - CFLAGS="-Ofast -pipe -fomit-frame-pointer -march=native"
-        - PYTHONHASHSEED=random
-        - PIP_UPGRADE_STRATEGY=eager
+  global:
+    - CC="ccache gcc"
+    - CCACHE_SLOPPINESS=file_macro,time_macros,include_file_ctime,include_file_mtime
+    - CCACHE_NOHASHDIR=true
+    - CFLAGS="-Ofast -pipe -fomit-frame-pointer -march=native"
+    - PYTHONHASHSEED=random
+    - PIP_UPGRADE_STRATEGY=eager
+    # Not Yet. See https://github.com/NextThought/nti.externalization/issues/105
+    # - ZOPE_INTERFACE_STRICT_IRO=1
+
 script:
   - coverage run -m zope.testrunner --test-path=src
 after_success:

CHANGES.rst

@@ -6,7 +6,15 @@
 1.1.2 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Adapt to a change in zope.container 4.4.0 that exposed unsafe
+  assumptions that ``providedBy(obj)`` would return the exact same
+  object with the exact same state on a subsequent call. This was
+  always a bug in the case of concurrency (e.g., if a different thread
+  called ``directlyProvides`` on the same object, or adjusted the
+  ``__bases__`` of some interface in the IRO); the zope.container
+  changes made it possible without concurrency. See
+  https://github.com/zopefoundation/zope.container/issues/38 and
+  https://github.com/NextThought/nti.externalization/issues/104.
 
 
 1.1.1 (2020-03-27)

setup.py

@@ -175,32 +175,32 @@ def _c(m):
     ext_modules=ext_modules,
     tests_require=TESTS_REQUIRE,
     install_requires=[
-        'setuptools',
-        'BTrees',
-        'isodate',
-        'nti.schema >= 1.7.0',
-        'persistent >= 4.4.3',
+        'BTrees >= 4.7.1',
         'PyYAML >= 5.1',
+        'ZODB >= 5.5.1',
+        'isodate',
+        'nti.schema >= 1.14.0',
+        'persistent >= 4.6.4',
         'pytz',
+        'setuptools',
         'simplejson',
-        'transaction >= 2.2',
         'six >= 1.11.0', # for the reference cycle fix in reraise()
-        'ZODB',
-        'zope.component',
-        'zope.configuration',
-        'zope.container',
-        'zope.dottedname',
-        'zope.dublincore',
-        'zope.event',
-        'zope.hookable >= 4.2.0',
-        'zope.interface >= 5.0.0', # getDirectTaggedValue
-        'zope.intid',
-        'zope.lifecycleevent',
-        'zope.location',
-        'zope.mimetype >= 2.3.0',
-        'zope.proxy',
-        'zope.schema >= 4.8.0',
-        'zope.security',
+        'transaction >= 2.2',
+        'zope.component >= 4.6.1',
+        'zope.configuration >= 4.4.0',
+        'zope.container >= 4.4.0',
+        'zope.dottedname >= 4.3.0',
+        'zope.dublincore >= 4.2.0',
+        'zope.event >= 4.4.0',
+        'zope.hookable >= 5.0.1',
+        'zope.interface >= 5.0.1', # getDirectTaggedValue
+        'zope.intid >= 4.3.0',
+        'zope.lifecycleevent >= 4.3.0',
+        'zope.location >= 4.2.0',
+        'zope.mimetype >= 2.5.0',
+        'zope.proxy >= 4.3.5',
+        'zope.schema >= 6.0.0',
+        'zope.security >= 5.1.1',
     ],
     extras_require={
         'test': TESTS_REQUIRE,

src/nti/externalization/__interface_cache.pxd

@@ -17,4 +17,4 @@ cdef class InterfaceCache(object):
 cdef _cache_cleanUp(instances)
 
 cpdef InterfaceCache cache_for(externalizer, ext_self)
-cpdef InterfaceCache cache_for_key(key, ext_self)
+cpdef InterfaceCache cache_for_key_in_providedBy(key, ext_self)

src/nti/externalization/_interface_cache.py

@@ -37,24 +37,18 @@ def __init__(self):
         self.ext_all_possible_keys = None
         self.ext_accept_external_id = None
         self.ext_primitive_out_ivars = None
+        # {'attrname': InterfaceDeclaringAttrname}
         self.modified_event_attributes = {}
 
 
-
-def cache_for_key(key, ext_self):
-    # The Declaration objects maintain a _v_attrs that
+def cache_for_key_in_providedBy(key, provided_by): # type: (object, object) -> InterfaceCache
+    # The Declaration objects returned from ``providedBy(obj)`` maintain a _v_attrs that
     # gets blown away on changes to themselves or their
     # dependents, including adding interfaces dynamically to an instance
     # (In that case, the provided object actually gets reset)
-    cache_place = providedBy(ext_self)
-    try:
-        attrs = cache_place._v_attrs # pylint:disable=protected-access
-        if attrs is None:
-            # _v_attrs became defined None to begin with in
-            # zope.interface 5
-            raise AttributeError
-    except AttributeError:
-        attrs = cache_place._v_attrs = {}
+    attrs = provided_by._v_attrs # pylint:disable=protected-access
+    if attrs is None:
+        attrs = provided_by._v_attrs = {}
 
     try:
         cache = attrs[key]
@@ -66,8 +60,8 @@ def cache_for_key(key, ext_self):
     return cache
 
 
-def cache_for(externalizer, ext_self):
-    return cache_for_key(type(externalizer), ext_self)
+def cache_for(externalizer, ext_self): # type: (object, object) -> InterfaceCache
+    return cache_for_key_in_providedBy(type(externalizer), providedBy(ext_self))
 
 
 def _cache_cleanUp(instances):

src/nti/externalization/internalization/_events.pxd

@@ -1,7 +1,7 @@
 # definitions for internalization.py
 import cython
 
-from nti.externalization.__interface_cache cimport cache_for_key
+from nti.externalization.__interface_cache cimport cache_for_key_in_providedBy
 
 # imports
 cdef providedBy

src/nti/externalization/internalization/_updater.pxd

@@ -68,6 +68,7 @@ cpdef update_from_external_object(containedObject,
                                   pre_hook=*)
 
 cdef dict _argspec_cacheg
+
 cdef str _UPDATE_ARGS_TWO
 cdef str _UPDATE_ARGS_ONE
 cdef str _UPDATE_ARGS_CONTEXT_KW

src/nti/externalization/internalization/events.py

@@ -17,7 +17,7 @@
 from zope.lifecycleevent import IAttributes
 
 from nti.externalization.interfaces import ObjectModifiedFromExternalEvent
-from nti.externalization._interface_cache import cache_for_key
+from nti.externalization._interface_cache import cache_for_key_in_providedBy
 
 logger = __import__('logging').getLogger(__name__)
 
@@ -43,47 +43,43 @@ def __init__(self, iface):
 
 
 def _make_modified_attributes(containedObject, external_keys):
-    cache = cache_for_key(_Attributes, containedObject)
-    # {'attrname': Interface}
-    attr_to_iface = cache.modified_event_attributes
+    # Returns a sequence of fresh _Attributes objects,
+    # one for each distinct interface (including None) that declared
+    # any key found in *external_keys*.
+
 
-    # Ensure that each attribute name in *names* is
-    # in the _v_attrs dict of *provides*. Also
-    # makes sure that self.attr_to_iface maps each
-    # name to its interface.
-    # Returns a sequence of fresh _Attributes objects.
+    # Get the InterfaceCache object. This object is reset when
+    # the interfaces implemented by containedObject are changed,
+    # such as by having bases added or removed, or by having directlyProvides/
+    # noLongerProvides/etc executed on it. In theory that could happen
+    # while this method is running, so access to internals like _v_attrs
+    # is not safe.
     provides = providedBy(containedObject)
-    # {iface -> _Attributes(iface)}
+    cache = cache_for_key_in_providedBy(_Attributes, provides)
+    # {'attrname': InterfaceDeclaringAttrname}; values may be None.
+    attr_to_iface = cache.modified_event_attributes
+
+    # {iface -> _Attributes(iface)}. Note that iface will be None if there
+    # is no interface that defined the key.
     result = {}
-    # By the time we get here, we're guaranteed that
-    # _v_attrs exists---it's where we're cached
-    attrs = provides._v_attrs # pylint:disable=protected-access
 
-    for name in external_keys:
-        # This is the core loop of provides.get()
-        # (z.i.declaration.Declaration.get)
-        attr = attrs.get(name, cache)
-        # Steady state, misses are rare here.
-        if attr is cache:
-            for iface in provides.__iro__:
-                attr = iface.direct(name)
-                if attr is not None:
-                    attrs[name] = attr
-                    break
-            else:
-                # Not part of any interface
-                attr = attrs[name] = None
+    provides_get = provides.get
 
+    for name in external_keys:
         providing_iface = attr_to_iface.get(name, cache)
         if providing_iface is cache:
-            # In the steady state, misses should be rare here.
+            # Ok, we haven't seen it before. Try to find it. Note that
+            # provides.get() (z.i.declaration.Declaration.get()) is
+            # only a positive cache, not a negative cache (if it would
+            # return None, the search up the IRO is repeated). So
+            # that's why we check our own cache first, because it is a
+            # negative cache.
             providing_iface = None
+            attr = provides_get(name)
             if attr is not None:
                 providing_iface = attr.interface
             attr_to_iface[name] = providing_iface
 
-
-
         attributes = result.get(providing_iface)
         if attributes is None:
             # Misses aren't unexpected here. often attributes

tox.ini

@@ -6,21 +6,15 @@ envlist =
 usedevelop = true
 commands =
     zope-testrunner --test-path=src []
+extras =
+    test
 deps =
      Cython >= 0.29
-    .[test]
-
-[testenv:py27-pure]
-basepython =
-    python2.7
 setenv =
-    PURE_PYTHON = 1
-
-[testenv:py36-pure]
-basepython =
-    python3.6
-setenv =
-    PURE_PYTHON = 1
+    pure: PURE_PYTHON=1
+# Not yet.
+# See https://github.com/NextThought/nti.externalization/issues/105
+#    ZOPE_INTERFACE_STRICT_IRO=1
 
 [testenv:coverage]
 usedevelop = true
@@ -31,7 +25,6 @@ commands =
     coverage html -i
     coverage report --fail-under=100
 deps =
-    {[testenv]deps}
     coverage
 setenv =
     PURE_PYTHON = 1