Re: Bug: Signal module create backreferences
Dominic LoBue <dom.lobue <at> gmail.com>
2010-04-05 01:20:21 GMT
On Sun, Apr 4, 2010 at 1:01 PM, Ian Ward <ian <at> excess.org> wrote:
> Dominic LoBue wrote on 2010-04-04 15:25:
>> On Sat, Mar 27, 2010 at 1:39 PM, Ian Ward <ian <at> excess.org> wrote:
>>> Dominic LoBue wrote on 2010-03-05 03:56:
>>>> Ian,
>>>>
>>>> I have a working implementation. By no means is it elegant, but it works.
>>>>
>>>> Basically instead of holding onto the specific method that is to be
>>>> called when a signal is recieved, we instead hold onto a tuple of
>>>> (ref(parentwithmethod), 'methodname'). When the signal is activated,
>>>> we use getattr to get an instance of the method we want and then run
>>>> that method.
>>>>
>>>> Working examples can be found here:
>>>> http://gist.github.com/322577
>>>> http://gist.github.com/322576
>>>>
>>> I just want to make sure I have a good understanding of what is
>>> happening with these references. The old way of tracking signals was a
>>> single WeakKeyDictionary:
>>>
>>> signals._connections = WeakKeyDictionary({
>>> object_that_sends_signal : {
>>> signal_name : [
>>> (callback_function, user_argument)]}})
>>>
>>> And in common use the callback_function is a bound method on the object
>>> that receives the signal.
>>>
>>> This receiving object may be the last thing holding a reference to the
>>> sending object, however when the receiving object is removed the bound
>>> method keeps it alive, in turn keeping the sending object alive and
>>> circular references in _connections are never removed.
>>>
>>> The same thing happens with the current code, except _connections is now
>>> in the sending object and nothing points to the dead objects, so I
>>> assume they would eventually get picked up by the garbage collector.
>>>
>>> (please correct me if I'm wrong)
>>>
>>
>> Ian,
>>
>> The solution actually has nothing to do with where the bound method is
>> being kept. Keeping the signal -> method mapping in one place is
>> preference; I think it is a cleaner solution overall.
>
> I agree with the preference for keeping all the mappings in one place.
> In my (new) tests, I haven't been able to make an implementation with
> everything together in one place work:
> http://excess.org/urwid/browser/reference_test.py
>
> Specifically the last test always fails and Urwid leaks memory. *
>
>> The problem is that bound methods reference the class they are bound
>> to, and in so doing they keep that class alive. The WeakKeyDictionary
>> doesn't do anything (at least in my case).
>
> s/class/object
>
>> To give an example, say the object_that_sends_signal from your example
>> is a property of object_that_receives_signal, and callback_function is
>> a bound method of object_that_receives_signal. This is a circular
>> reference: object_that_receives_signal references
>> object_that_sends_signal, keeping the dictionary entries alive; and
>> the bound method callback_function references
>> object_that_receives_signal, keeping it alive as well. All the nesting
>> that the bound_method is below prevents the weak reference from
>> working.
>
> * Right, but maybe we're working under different assumptions. I think
> it's ok for the only reference to callback function to be via a signal.
> Won't your code delete the object_that_receives_signal if the only
> reference to object_that_receives_signal is from a signal?
>
Nope. That's what the script I included in my first email proved.
>>> One problem with your solution is that it only helps if the
>>> callback_function is a bound method. What if it's a normal function
>>> that happened to pull in a reference to the sending object from its
>>> enclosing scope?
>>
>> Can you give an example of what you mean?
>>
>
> Idle speculation, I haven't made it work(fail?) yet. Something along
> the lines of:
>
> def setup_my_signal_handler():
> foo = ObjectThatSendsSignal()
> def signal_handler(f):
> print foo, f
> urwid.connect_signal(foo, 'some_signal', signal_handler)
>
> where signal_handler() has a reference to foo inherited from the
> enclosing function.
>
Ohh, cool. I'll have to try that.
> Ian
>
> _______________________________________________
> Urwid mailing list
> Urwid <at> lists.excess.org
> http://lists.excess.org/mailman/listinfo/urwid
>
Okay, just tried it using my fix and I get a bug where the nested
function is released and the weak reference breaks:
python circularsignals.py
child phones home
Traceback (most recent call last):
File "circularsignals.py", line 27, in <module>
a.something.do_emit()
File "circularsignals.py", line 8, in do_emit
emit_signal(self, 'something')
File "/tank/projects/urwid/urwid/signals.py", line 77, in emit
result |= bool(callback(*args_copy))
ReferenceError: weakly-referenced object no longer exists
For my test I just modified the "my_object" class from the code in my
original email to this:
class my_object(object):
__metaclass__ = MetaSignals
signals = ['something']
def __init__(self):
def nest_func():
print "nested!"
self.something = my_other_object()
connect_signal(self.something, 'something', self.melse)
connect_signal(self.something, 'something', nest_func)
def melse(self):
print 'child phones home'
I fixed the ducktype implementation to not proxy anything other than
methods and pushed it to my branch.
--
--
Dominic LoBue