diff --git a/.travis.yml b/.travis.yml index a62bd4d..66a53a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,12 +2,12 @@ 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 @@ -15,13 +15,16 @@ jobs: - 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: diff --git a/CHANGES.rst b/CHANGES.rst index 632f5a6..16d2e23 100644 --- a/CHANGES.rst +++ b/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) diff --git a/setup.py b/setup.py index 114cdba..d2a4362 100755 --- a/setup.py +++ b/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, diff --git a/src/nti/externalization/__interface_cache.pxd b/src/nti/externalization/__interface_cache.pxd index ba38726..935167c 100644 --- a/src/nti/externalization/__interface_cache.pxd +++ b/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) diff --git a/src/nti/externalization/_interface_cache.py b/src/nti/externalization/_interface_cache.py index f2a2b1f..edaec3d 100644 --- a/src/nti/externalization/_interface_cache.py +++ b/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): diff --git a/src/nti/externalization/internalization/_events.pxd b/src/nti/externalization/internalization/_events.pxd index c99631e..50bf80e 100644 --- a/src/nti/externalization/internalization/_events.pxd +++ b/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 diff --git a/src/nti/externalization/internalization/_updater.pxd b/src/nti/externalization/internalization/_updater.pxd index 2b15ec9..99457d8 100644 --- a/src/nti/externalization/internalization/_updater.pxd +++ b/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 diff --git a/src/nti/externalization/internalization/events.py b/src/nti/externalization/internalization/events.py index bfbf3cd..2ffa245 100644 --- a/src/nti/externalization/internalization/events.py +++ b/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 diff --git a/tox.ini b/tox.ini index d3dcc4b..9f9e7e0 100644 --- a/tox.ini +++ b/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