zopefoundation / zope.interface

Interfaces for Python
http://zopeinterface.readthedocs.io/
Other
325 stars 71 forks source link

BaseAdapterRegistry.subscribe/unsubscribe non-symmetric updating `_provided`, leaves orphan entries #227

Closed jamadden closed 3 years ago

jamadden commented 3 years ago

While working on #224 I noticed that there were no tests for BaseAdapterRegistry._provided (which keeps a map of {interface: reference count} of interfaces it knows about in order to add them as "extendors" to the AdapterLookup classes).

The register method increments this count and stores it: https://github.com/zopefoundation/zope.interface/blob/bf0de08af2d53261d7eb0ce1e31f574f93524ef6/src/zope/interface/adapter.py#L154-L157 As does the subscribe method: https://github.com/zopefoundation/zope.interface/blob/bf0de08af2d53261d7eb0ce1e31f574f93524ef6/src/zope/interface/adapter.py#L247-L251

The inverse of register, unregister, decrements this count and either stores it, or, if it hits 0, removes it: https://github.com/zopefoundation/zope.interface/blob/bf0de08af2d53261d7eb0ce1e31f574f93524ef6/src/zope/interface/adapter.py#L219-L224

However, the inverse of subscribe, unsubscribe, appears to have a bug. It decrements the count, but only if it hits zero is it removed. It never stores it, which means it can never hit 0 just by unsubscribing. https://github.com/zopefoundation/zope.interface/blob/bf0de08af2d53261d7eb0ce1e31f574f93524ef6/src/zope/interface/adapter.py#L305-L309

Making unsubscribe store the decremented value doesn't break any tests (and fixes the tests I was adding). Because the high-level class Components automatically both registers and subscribes utilities, it's fairly easy to end up with orphaned entries in _provided.

Here, I'll define an interface and class and register it both as the default (unnamed) and a named utility for the same interface.

>>> from zope.interface import Interface, implementer
>>> from zope.interface.registry import Components
>>> class IFoo(Interface):
...     pass
...
>>> @implementer(IFoo)
... class Foo:
...     pass
...
>>> foo = Foo()
>>> foo2 = Foo()
>>> comp = Components()
>>> comp.registerUtility(foo)
None
>>> comp.registerUtility(foo2, name='bar')
None

Now, because IFoo was both provided and subscribed twice, it has a refcount of 4, and the two objects show up once each in _adapters and _subscribers of the adapter registry:

>>> comp.utilities._provided
{<InterfaceClass __main__.IFoo>: 4}
>>> comp.utilities._adapters
[{<InterfaceClass __main__.IFoo>: {'': <__main__.Foo object at 0x10a09dfa0>,
                                   'bar': <__main__.Foo object at 0x10a08e140>}}]
>>> comp.utilities._subscribers
[{<InterfaceClass __main__.IFoo>: {'': (<__main__.Foo object at 0x10a09dfa0>,
                                        <__main__.Foo object at 0x10a08e140>)}}]

Now we can unregister both utilities, and their entries will be gone from _adapters and _subscribers:

>>> comp.unregisterUtility(foo)
True
>>> comp.unregisterUtility(foo2, name='bar')
True
>>> comp.utilities._adapters, comp.utilities._subscribers
([], [])

But IFoo is still in _provided because of this issue:

>>> comp.utilities._provided
{<InterfaceClass __main__.IFoo>: 2}

Because of the way unsubscribe calculates the delta (removing all equal objects) if I register the same identical object (which is if course equal to itself) as both the named and unnamed utility, this appears to work. That is, _provided has no remaining references to IFoo.

When using non-equal objects, this only works (empty _provided) if I make unsubscribe store the decremented value. The equal object case still works then.