Comment 0 for bug 98474

Uploaded: persistent-interfaces-tests.diff

zope.app.interface.PersistentInterfaceClass uses a PersistentDict for
the dependents attribute instead of the WeakKeyDictionary used in
zope.interface.interface.InterfaceClass. There are a number of bugs
associated with this approach not exposed in any tests. Attached is a
diff to the tests of zope.app.interface and zope.app.component to
expose these bugs.

Firstly, if IFoo is a zope.app.interface.PersistentInterfaceClass
instance and zope.interface.directlyProvides(foo, IFoo), the
declaraion can't be verified from a different ZODB connection with
IFoo.providedBy(foo) due to the following. When the ZODB is trying to
unpickle the dependents attribute of IFoo, it procedes down the
serialization of IFoo to the ProvidesClass instance representing the
declaration. It begins reconstituting the ProvidesClass instance,
which calls IFoo.subscribe(provides) which accesses the dependents
attribute of IFoo. Since the ProvidesClass instance isn't persistent,
it has no oid the ZODB circular reference check doesn't catch this
circle. As a result the fix is to make sure that the declarations
instances in the dependents attribute are themselves persistent.

Also attached is a diff to zope.app.interface that replaces
PersistentInterfaceClass.dependents with a custom dict that converts
non-persistent declarations being added to the dependents attribute
into persistent versions of the same.

This fixes the problem but is not optimal because there are then two
instances for the same declaration, one in the ProvidesClass instance
stored in the object's __provides__ attribute and the other in the
dependents attribute of the PersistentInterfaceClass.

It seems like the more appropriate solution would be to check for
PersistentInterfaceClass instances in
zope.interface.declarations.directlyProvides and use the persistent
declaration classes for those declaraions. Since
PersistentInterfaceClass is in zope.app.interface and zope.interface,
I wasn't sure which was the lesser of the two evils, so I restrained
my changes to zope.app. What might be a better solution? Give me
some feedback and I'll change the implementation.

There's another problem with the
zope.app.interface.PersistentInterfaceClass.dependents attribute.
zope.interface.InterfaceClass.dependents is a WeakKeyDictionary so
that declarations don't keep objects from being freed if the object is
removed. By using a PersistentDict for dependents, the declarations
can keep an object from being freed from memory and/or the ZODB when
the object is removed.

My first patch also includes tests for this bug. These tests seem to
have exposed another unrelated bug. An instance of a class that
subclasses persistent.Persistent is declared that it
zope.interface.directlyProvides a zope.interface.InterfaceClass
instance and then the persistent object is added to the ZODB and
committed. Then if the persistent instance is deleted from the ZODB,
the transaction is committed, and the ZODB is packed, and gc.collect
is run, the ProvidesClass instance in the InterfaceClass instance
still remains. It does not, however, remain if the persistent
interface was never added to the ZODB.

I'm not sure if this represents a potential memory leak or not. What
confuses me is that it all behaves properly unless the persistent
instance is added to the ZODB. I noted the comment about weak
referrences being added to the ZODB optomistically in ZODB.serialize.
Could that be it? Unfortunately, my second patch doesn't include a
fix for this. I'd be happy to investigate this further if given a
little direction.