[sugar] Ticket #1289, patch review

Marco Pesenti Gritti mpg
Sat Apr 7 05:22:36 EDT 2007


On Sat, 2007-04-07 at 00:34 -0400, Mike C. Fletcher wrote:
> Marco Pesenti Gritti wrote:
> > First of all this is really useful, thanks!
> >   
> Most welcome.
> > +        # XXX this creates an uncollectable object, should be handled
> > +        # using a weakref callback...
> >          gobject.source_remove(self._launch_timeout_id)
> >          self._launch_timeout_id = 0
> >
> > I don't understand this. To which object are you referring?
> >   
> The current object, in this instance, a HomeActivity.  When you define a 
> __del__ method you prevent Python's garbage collection system from 
> collecting cycles that happen to include this object.  Generally 
> speaking, you shouldn't use __del__ for this kind of cleanup work, 
> instead use a weakref.ref with a callable that will clean up after the 
> object (without any reference to the attributes of the object).  It 
> takes more work (because by the time it is called the object is already 
> gone), but in complex systems it tends to produce fewer hard-to-debug 
> memory leaks.
> 

Oh, I was looking at the implementation, not at the method itself. Ok,
good to know. There are actually several places where we are using
__del__ for cleanup.

> > +        """Retrieve the "activity_id" passed in to our constructor
> > +        
> > +        XXX What is the activity id, really?
> >
> > Each activity instance has a unique id, so that it can be resumed and
> > shared on the network.

You can see how it's constructed in sugar/util.py but... I don't think
that's set in stones yet. Dan?
   
> Nope, don't have access yet.  Will email Ivan.

Cool, please check this in when you have access. There is a comment
about setting the dpi that should be removed (because the code is gone).

Cheers,
Marco




More information about the Sugar-devel mailing list