[MERGE] Transport support for pack repositories

Andrew Bennetts andrew at canonical.com
Thu Aug 9 02:34:20 BST 2007


Martin Pool wrote:
[...]
> 
> > That gets into __del__ or exit hooks; I'm really not a fan of these,
> > because of the various vaguaries of python and memory management. More
> > importantly here, we are using these streams to allow incremental pack
> > writing *to an upload directory*. The 'write place' :) to do such a
> > check would actually be the rename() call, so that you don't rename an
> > open stream.
> 
> I think the returned object should have a __del__ method that warns
> "stream was not closed".

A trick you can use to limit the risk of __del__ methods is to put the __del__
method on an object that is stored as an attribute of the object you want to
warn about.  e.g.:


    class Foo(object):

        def __init__(self):
            self.__warn_on_del = _WarnOnDel()

        def start(self):
            ...
            self.__warn_on_del.started = 'description of this Foo instance'

        def stop(self):
            # We've been stopped cleanly, so don't warn when this instance is
            # garbage collected.
            self.__warn_on_del.started = None


    class _WarnOnDel(object):

        def __init__(self):
            self.started = None

        def __del__(self):
            if self.started is not None:
                trace.warn('Foo %r was not stopped.' % (self.started,))


Obviously the _WarnOnDel instances should have exactly one reference, in Foo,
to avoid any risk of a cycle.  Also, care needs to be taken about what objects
are passed to _WarnOnDel to avoid a cycle.  If a string description can be
generated that is ideal, as that is clearly safe.

This won't affect the lifetime of Foo instances.  When a Foo instance is
collected, the object in its __warn_on_del attribute will have a refcount of 0,
and so the _WarnOnDel's __del__ method will be triggered.  Thus if a Foo is
start()ed and garbage-collected before being stop()ed, a warning will be
produced.

Twisted does this with its Deferred objects (optionally; the debug info it
generates is expensive so this behaviour is off by default), and it works well.

I'm not 100% certain that this is watertight on Python implementations other
than CPython... but then I don't think Jython or IronPython can run bzr at the
moment anyway.  (Jython still hasn't made a release that's caught up to Python
2.4 last I checked, and IronPython still has a bunch of incompatibilities like
its str/unicode unification.)

-Andrew.




More information about the bazaar mailing list