I've submitted some new tests and a fix for zope.app.interface.PersistentInterfaceClass in:
http://www.zope.org/Collectors/Zope3-dev/747 I have commit priveleges to the svn repo, but I'd love to get some review before commiting. baijum from #zope3-dev suggested I post here. Below is a copy of the text from issue 747. > 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. Ross _______________________________________________ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
