Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C/Python impl difference: C impl of IFoo.__call__ swallows *all* exceptions accessing __conform__ #163

Closed
jamadden opened this issue Jan 28, 2020 · 1 comment
Labels

Comments

@jamadden
Copy link
Member

While investigating #162, I found a discrepancy in the C and Python implementations of InterfaceBase.__call__. In C, if accessing the __conform__ attribute returns NULL, meaning an exception was raised, we ignore it (call PyErr_Clear) and continue on. In the Python version, though, we just use getattr(obj, '__conform__', None). In Python 3, that only catches AttributeError.

This matters because of things like Acquisition which can raise strange exceptions. In particular, the test cases for five.intid have a setup that with a cycle in the acquisition path. That raises a RuntimeError (at least with the Python implementation) and the test fails with the RuntimeError, instead of continuing with the normal adapter lookup.

Ideally, the C implementation wouldn't be catching and discarding arbitrary exceptions (MemoryError, KeyboardInterrupt, etc), but that ship has probably sailed. So probably the best bet is to make the Python implementation also catch all exceptions. Sigh.

@jamadden jamadden added the bug label Jan 28, 2020
@jamadden
Copy link
Member Author

The same thing goes for providedBy: The C implementation catches all exception, the Python version sensibly catches just AttributeError when accessing __providedBy__.

And then the same thing goes for getObjectSpecification accessing __provides__ and __class__.

@jamadden jamadden reopened this Feb 5, 2020
@jamadden jamadden closed this as completed Feb 5, 2020
admin-turris referenced this issue in CZ-NIC/turris-os-packages Jul 10, 2020
5.1.0 (2020-04-08)
==================

- Make ``@implementer(*iface)`` and ``classImplements(cls, *iface)``
  ignore redundant interfaces. If the class already implements an
  interface through inheritance, it is no longer redeclared
  specifically for *cls*. This solves many instances of inconsistent
  resolution orders, while still allowing the interface to be declared
  for readability and maintenance purposes. See `issue 199
  <https://github.com/zopefoundation/zope.interface/issues/199>`_.

- Remove all bare ``except:`` statements. Previously, when accessing
  special attributes such as ``__provides__``, ``__providedBy__``,
  ``__class__`` and ``__conform__``, this package wrapped such access
  in a bare ``except:`` statement, meaning that many errors could pass
  silently; typically this would result in a fallback path being taken
  and sometimes (like with ``providedBy()``) the result would be
  non-sensical. This is especially true when those attributes are
  implemented with descriptors. Now, only ``AttributeError`` is
  caught. This makes errors more obvious.

  Obviously, this means that some exceptions will be propagated
  differently than before. In particular, ``RuntimeError`` raised by
  Acquisition in the case of circular containment will now be
  propagated. Previously, when adapting such a broken object, a
  ``TypeError`` would be the common result, but now it will be a more
  informative ``RuntimeError``.

  In addition, ZODB errors like ``POSKeyError`` could now be
  propagated where previously they would ignored by this package.

  See `issue 200 <https://github.com/zopefoundation/zope.interface/issues/200>`_.

- Require that the second argument (*bases*) to ``InterfaceClass`` is
  a tuple. This only matters when directly using ``InterfaceClass`` to
  create new interfaces dynamically. Previously, an individual
  interface was allowed, but did not work correctly. Now it is
  consistent with ``type`` and requires a tuple.

- Let interfaces define custom ``__adapt__`` methods. This implements
  the other side of the :pep:`246` adaptation protocol: objects being
  adapted could already implement ``__conform__`` if they know about
  the interface, and now interfaces can implement ``__adapt__`` if
  they know about particular objects. There is no performance penalty
  for interfaces that do not supply custom ``__adapt__`` methods.

  This includes the ability to add new methods, or override existing
  interface methods using the new ``@interfacemethod`` decorator.

  See `issue 3 <https://github.com/zopefoundation/zope.interface/issues/3>`_.

- Make the internal singleton object returned by APIs like
  ``implementedBy`` and ``directlyProvidedBy`` for objects that
  implement or provide no interfaces more immutable. Previously an
  internal cache could be mutated. See `issue 204
  <https://github.com/zopefoundation/zope.interface/issues/204>`_.

5.0.2 (2020-03-30)
==================

- Ensure that objects that implement no interfaces (such as direct
  subclasses of ``object``) still include ``Interface`` itself in
  their ``__iro___`` and ``__sro___``. This fixes adapter registry
  lookups for such objects when the adapter is registered for
  ``Interface``. See `issue 197
  <https://github.com/zopefoundation/zope.interface/issues/197>`_.

5.0.1 (2020-03-21)
==================

- Ensure the resolution order for ``InterfaceClass`` is consistent.
  See `issue 192 <https://github.com/zopefoundation/zope.interface/issues/192>`_.

- Ensure the resolution order for ``collections.OrderedDict`` is
  consistent on CPython 2. (It was already consistent on Python 3 and PyPy).

- Fix the handling of the ``ZOPE_INTERFACE_STRICT_IRO`` environment
  variable. Previously, ``ZOPE_INTERFACE_STRICT_RO`` was read, in
  contrast with the documentation. See `issue 194
  <https://github.com/zopefoundation/zope.interface/issues/194>`_.

5.0.0 (2020-03-19)
==================

- Make an internal singleton object returned by APIs like
  ``implementedBy`` and ``directlyProvidedBy`` immutable. Previously,
  it was fully mutable and allowed changing its ``__bases___``. That
  could potentially lead to wrong results in pathological corner
  cases. See `issue 158
  <https://github.com/zopefoundation/zope.interface/issues/158>`_.

- Support the ``PURE_PYTHON`` environment variable at runtime instead
  of just at wheel build time. A value of 0 forces the C extensions to
  be used (even on PyPy) failing if they aren't present. Any other
  value forces the Python implementation to be used, ignoring the C
  extensions. See `PR 151 <https://github.com/zopefoundation/zope.interface/pull/151>`_.

- Cache the result of ``__hash__`` method in ``InterfaceClass`` as a
  speed optimization. The method is called very often (i.e several
  hundred thousand times during Plone 5.2 startup). Because the hash value never
  changes it can be cached. This improves test performance from 0.614s
  down to 0.575s (1.07x faster). In a real world Plone case a reindex
  index came down from 402s to 320s (1.26x faster). See `PR 156
  <https://github.com/zopefoundation/zope.interface/pull/156>`_.

- Change the C classes ``SpecificationBase`` and its subclass
  ``ClassProvidesBase`` to store implementation attributes in their structures
  instead of their instance dictionaries. This eliminates the use of
  an undocumented private C API function, and helps make some
  instances require less memory. See `PR 154 <https://github.com/zopefoundation/zope.interface/pull/154>`_.

- Reduce memory usage in other ways based on observations of usage
  patterns in Zope (3) and Plone code bases.

  - Specifications with no dependents are common (more than 50%) so
    avoid allocating a ``WeakKeyDictionary`` unless we need it.
  - Likewise, tagged values are relatively rare, so don't allocate a
    dictionary to hold them until they are used.
  - Use ``__slots___`` or the C equivalent ``tp_members`` in more
    common places. Note that this removes the ability to set arbitrary
    instance variables on certain objects.
    See `PR 155 <https://github.com/zopefoundation/zope.interface/pull/155>`_.

  The changes in this release resulted in a 7% memory reduction after
  loading about 6,000 modules that define about 2,200 interfaces.

  .. caution::

     Details of many private attributes have changed, and external use
     of those private attributes may break. In particular, the
     lifetime and default value of ``_v_attrs`` has changed.

- Remove support for hashing uninitialized interfaces. This could only
  be done by subclassing ``InterfaceClass``. This has generated a
  warning since it was first added in 2011 (3.6.5). Please call the
  ``InterfaceClass`` constructor or otherwise set the appropriate
  fields in your subclass before attempting to hash or sort it. See
  `issue 157 <https://github.com/zopefoundation/zope.interface/issues/157>`_.

- Remove unneeded override of the ``__hash__`` method from
  ``zope.interface.declarations.Implements``. Watching a reindex index
  process in ZCatalog with on a Py-Spy after 10k samples the time for
  ``.adapter._lookup`` was reduced from 27.5s to 18.8s (~1.5x faster).
  Overall reindex index time shrunk from 369s to 293s (1.26x faster).
  See `PR 161
  <https://github.com/zopefoundation/zope.interface/pull/161>`_.

- Make the Python implementation closer to the C implementation by
  ignoring all exceptions, not just ``AttributeError``, during (parts
  of) interface adaptation. See `issue 163
  <https://github.com/zopefoundation/zope.interface/issues/163>`_.

- Micro-optimization in ``.adapter._lookup`` , ``.adapter._lookupAll``
  and ``.adapter._subscriptions``: By loading ``components.get`` into
  a local variable before entering the loop a bytcode "LOAD_FAST 0
  (components)" in the loop can be eliminated. In Plone, while running
  all tests, average speedup of the "owntime" of ``_lookup`` is ~5x.
  See `PR 167
  <https://github.com/zopefoundation/zope.interface/pull/167>`_.

- Add ``__all__`` declarations to all modules. This helps tools that
  do auto-completion and documentation and results in less cluttered
  results. Wildcard ("*") are not recommended and may be affected. See
  `issue 153
  <https://github.com/zopefoundation/zope.interface/issues/153>`_.

- Fix ``verifyClass`` and ``verifyObject`` for builtin types like
  ``dict`` that have methods taking an optional, unnamed argument with
  no default value like ``dict.pop``. On PyPy3, the verification is
  strict, but on PyPy2 (as on all versions of CPython) those methods
  cannot be verified and are ignored. See `issue 118
  <https://github.com/zopefoundation/zope.interface/issues/118>`_.

- Update the common interfaces ``IEnumerableMapping``,
  ``IExtendedReadMapping``, ``IExtendedWriteMapping``,
  ``IReadSequence`` and ``IUniqueMemberWriteSequence`` to no longer
  require methods that were removed from Python 3 on Python 3, such as
  ``__setslice___``. Now, ``dict``, ``list`` and ``tuple`` properly
  verify as ``IFullMapping``, ``ISequence`` and ``IReadSequence,``
  respectively on all versions of Python.

- Add human-readable ``__str___`` and ``__repr___`` to ``Attribute``
  and ``Method``. These contain the name of the defining interface
  and the attribute. For methods, it also includes the signature.

- Change the error strings raised by ``verifyObject`` and
  ``verifyClass``. They now include more human-readable information
  and exclude extraneous lines and spaces. See `issue 170
  <https://github.com/zopefoundation/zope.interface/issues/170>`_.

  .. caution:: This will break consumers (such as doctests) that
               depended on the exact error messages.

- Make ``verifyObject`` and ``verifyClass`` report all errors, if the
  candidate object has multiple detectable violations. Previously they
  reported only the first error. See `issue
  <https://github.com/zopefoundation/zope.interface/issues/171>`_.

  Like the above, this will break consumers depending on the exact
  output of error messages if more than one error is present.

- Add ``zope.interface.common.collections``,
  ``zope.interface.common.numbers``, and ``zope.interface.common.io``.
  These modules define interfaces based on the ABCs defined in the
  standard library ``collections.abc``, ``numbers`` and ``io``
  modules, respectively. Importing these modules will make the
  standard library concrete classes that are registered with those
  ABCs declare the appropriate interface. See `issue 138
  <https://github.com/zopefoundation/zope.interface/issues/138>`_.

- Add ``zope.interface.common.builtins``. This module defines
  interfaces of common builtin types, such as ``ITextString`` and
  ``IByteString``, ``IDict``, etc. These interfaces extend the
  appropriate interfaces from ``collections`` and ``numbers``, and the
  standard library classes implement them after importing this module.
  This is intended as a replacement for third-party packages like
  `dolmen.builtins <https://pypi.org/project/dolmen.builtins/>`_.
  See `issue 138 <https://github.com/zopefoundation/zope.interface/issues/138>`_.

- Make ``providedBy()`` and ``implementedBy()`` respect ``super``
  objects. For instance, if class ``Derived`` implements ``IDerived``
  and extends ``Base`` which in turn implements ``IBase``, then
  ``providedBy(super(Derived, derived))`` will return ``[IBase]``.
  Previously it would have returned ``[IDerived]`` (in general, it
  would previously have returned whatever would have been returned
  without ``super``).

  Along with this change, adapter registries will unpack ``super``
  objects into their ``__self___`` before passing it to the factory.
  Together, this means that ``component.getAdapter(super(Derived,
  self), ITarget)`` is now meaningful.

  See `issue 11 <https://github.com/zopefoundation/zope.interface/issues/11>`_.

- Fix a potential interpreter crash in the low-level adapter
  registry lookup functions. See issue 11.

- Adopt Python's standard `C3 resolution order
  <https://www.python.org/download/releases/2.3/mro/>`_ to compute the
  ``__iro__`` and ``__sro__`` of interfaces, with tweaks to support
  additional cases that are common in interfaces but disallowed for
  Python classes. Previously, an ad-hoc ordering that made no
  particular guarantees was used.

  This has many beneficial properties, including the fact that base
  interface and base classes tend to appear near the end of the
  resolution order instead of the beginning. The resolution order in
  general should be more predictable and consistent.

  .. caution::
     In some cases, especially with complex interface inheritance
     trees or when manually providing or implementing interfaces, the
     resulting IRO may be quite different. This may affect adapter
     lookup.

  The C3 order enforces some constraints in order to be able to
  guarantee a sensible ordering. Older versions of zope.interface did
  not impose similar constraints, so it was possible to create
  interfaces and declarations that are inconsistent with the C3
  constraints. In that event, zope.interface will still produce a
  resolution order equal to the old order, but it won't be guaranteed
  to be fully C3 compliant. In the future, strict enforcement of C3
  order may be the default.

  A set of environment variables and module constants allows
  controlling several aspects of this new behaviour. It is possible to
  request warnings about inconsistent resolution orders encountered,
  and even to forbid them. Differences between the C3 resolution order
  and the previous order can be logged, and, in extreme cases, the
  previous order can still be used (this ability will be removed in
  the future). For details, see the documentation for
  ``zope.interface.ro``.

- Make inherited tagged values in interfaces respect the resolution
  order (``__iro__``), as method and attribute lookup does. Previously
  tagged values could give inconsistent results. See `issue 190
  <https://github.com/zopefoundation/zope.interface/issues/190>`_.

- Add ``getDirectTaggedValue`` (and related methods) to interfaces to
  allow accessing tagged values irrespective of inheritance. See
  `issue 190
  <https://github.com/zopefoundation/zope.interface/issues/190>`_.

- Ensure that ``Interface`` is always the last item in the ``__iro__``
  and ``__sro__``. This is usually the case, but if classes that do
  not implement any interfaces are part of a class inheritance
  hierarchy, ``Interface`` could be assigned too high a priority.
  See `issue 8 <https://github.com/zopefoundation/zope.interface/issues/8>`_.

- Implement sorting, equality, and hashing in C for ``Interface``
  objects. In micro benchmarks, this makes those operations 40% to 80%
  faster. This translates to a 20% speed up in querying adapters.

  Note that this changes certain implementation details. In
  particular, ``InterfaceClass`` now has a non-default metaclass, and
  it is enforced that ``__module__`` in instances of
  ``InterfaceClass`` is read-only.

  See `PR 183 <https://github.com/zopefoundation/zope.interface/pull/183>`_.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant