Traits memory leak with DelegatesTo

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Traits memory leak with DelegatesTo

Scott Swarts

I've run into a memory leak when using DelegatesTo traits. An object that delegates to another object appears to live as long as the object it delegates to lives, even if there are no other references to the object.  For example, the following script should not print any Delegates objects, but it does:

from enthought.traits.api import *

class Base(HasTraits):
    """ Object we are delegating to. """

    i = Int


class Delegates(HasTraits):
    """ Object that delegates. """

    b = Instance(Base)

    i = DelegatesTo('b')

# Make a pair of objects
b = Base()
d = Delegates(b=b)

# Delete d and thoroughly collect garbage
del d
import gc
from pprint import pprint
for i in range(3):
    gc.collect(2)

# See if we still have a Delegates
ds = [ obj for obj in gc.get_objects() if isinstance(obj, Delegates) ]
pprint(ds)
I've traced down this problem to a problem with closures as trait handlers. In has_traits.py we have the following code:

    def _init_trait_delegate_listener ( self, name, kind, pattern ):
        """ Sets up the listener for a delegate trait.
        """
        name_pattern    = self._trait_delegate_name( name, pattern )
        target_name_len = len( name_pattern.split( ':' )[-1] )
       
        def notify ( object, notify_name, old, new ):
            self.trait_property_changed( name + notify_name[ target_name_len: ],
                                         old, new )
           
        self.on_trait_change( notify, name_pattern )
        self.__dict__.setdefault( ListenerTraits, {} )[ name ] = notify
       


'notify' is a closure and contains a reference to self, which in the above example is 'd'. The on_trait_change call adds a handler on 'b' which contains a reference to the closure. Since 'd' has a reference to 'b', this ends up creating a reference cycle so that 'd' will not be freed as long as 'b' lives.  When the handler is a bound method traits is careful to use a weak reference to the object to avoid cycles like this, but in this case traits doesn't know there is a reference to 'd' hidden in the closure.

This leak is a major problem for us, so we propose fixing it as follows:

1. Add an optional 'target' argument to on_trait_change. If target is specified and the handler is a function (or closure), the handler will automatically be removed when the target object is deleted.

2. Modify the 'notify' closure to use a weak reference to self.  To make this easier we will add a decorator called 'weak_arg' that creates a weak reference and then deferences it before calling the function.  The above code would then be:

    def _init_trait_delegate_listener ( self, name, kind, pattern ):
        """ Sets up the listener for a delegate trait.
        """
        name_pattern    = self._trait_delegate_name( name, pattern )
        target_name_len = len( name_pattern.split( ':' )[-1] )

        @weak_arg(self)
        def notify ( self, object, notify_name, old, new ):
            self.trait_property_changed( name + notify_name[ target_name_len: ],
                                         old, new )
           
        self.on_trait_change( notify, name_pattern, target=self )
        self.__dict__.setdefault( ListenerTraits, {} )[ name ] = notify

There are several other closures used in has_traits.py that may have similar problems. We would change these as well.

Does anyone have any feedback on this proposal? 

Thanks,

Scott





_______________________________________________
Enthought-Dev mailing list
[hidden email]
https://mail.enthought.com/mailman/listinfo/enthought-dev
Reply | Threaded
Open this post in threaded view
|

Re: Traits memory leak with DelegatesTo

Peter Wang
On 2/15/10 5:08 PM, Scott Swarts wrote:

> 2. Modify the 'notify' closure to use a weak reference to self. To make
> this easier we will add a decorator called 'weak_arg' that creates a
> weak reference and then deferences it before calling the function. The
> above code would then be:
>
> def _init_trait_delegate_listener ( self, name, kind, pattern ):
> """ Sets up the listener for a delegate trait.
> """
> name_pattern = self._trait_delegate_name( name, pattern )
> target_name_len = len( name_pattern.split( ':' )[-1] )
>
> @weak_arg(self)
> def notify ( self, object, notify_name, old, new ):

Are you using this trick to find the right cell in the closure?

http://74.125.95.132/search?q=cache:ef5tewT-AYcJ:code.activestate.com/recipes/439096/+python+closure+cell&cd=1&hl=en&ct=clnk&gl=us

(I'm linking to the Google cache b/c code.activestate.com is down right
now.)

-Peter
_______________________________________________
Enthought-Dev mailing list
[hidden email]
https://mail.enthought.com/mailman/listinfo/enthought-dev
Reply | Threaded
Open this post in threaded view
|

Re: Traits memory leak with DelegatesTo

Scott Swarts

No, I'm making it so 'self' is not saved in the closure. I added an extra arg and am passing it in. 

Scott

Peter Wang wrote:
On 2/15/10 5:08 PM, Scott Swarts wrote:

  
2. Modify the 'notify' closure to use a weak reference to self. To make
this easier we will add a decorator called 'weak_arg' that creates a
weak reference and then deferences it before calling the function. The
above code would then be:

def _init_trait_delegate_listener ( self, name, kind, pattern ):
""" Sets up the listener for a delegate trait.
"""
name_pattern = self._trait_delegate_name( name, pattern )
target_name_len = len( name_pattern.split( ':' )[-1] )

@weak_arg(self)
def notify ( self, object, notify_name, old, new ):
    

Are you using this trick to find the right cell in the closure?

http://74.125.95.132/search?q=cache:ef5tewT-AYcJ:code.activestate.com/recipes/439096/+python+closure+cell&cd=1&hl=en&ct=clnk&gl=us

(I'm linking to the Google cache b/c code.activestate.com is down right 
now.)

-Peter
_______________________________________________
Enthought-Dev mailing list
[hidden email]
https://mail.enthought.com/mailman/listinfo/enthought-dev
  


_______________________________________________
Enthought-Dev mailing list
[hidden email]
https://mail.enthought.com/mailman/listinfo/enthought-dev