[PATCH 2 of 2] keyword: eliminate potential reference cycles from kwrepo

[Adrian Buehlmann]

On 07.07.2009 17:59, Martin Geisler wrote:
> Christian Ebert <blacktrash at gmx.net> writes:
> 
> Hi Christian
> 
>> [...
>>
>> ATM there is a mixture of self.repomemberfunc and repo.repomemberfunc,
>> and when I tried to clean this up Matt chimed in like so:
>>
>> http://marc.info/?l=mercurial-devel&m=124654567201924&w=2
> 
> Ah, I had forgotten about that mail...
> 
>>>>         def kwcommitctx(self, ctx, error=False):
>>>>             wlock = lock = None
>>>>             try:
>>>> -                wlock = self.wlock()
>>>> -                lock = self.lock()
>>>> +                wlock = super(kwrepo, self).wlock()
>>>> +                lock = super(kwrepo, self).lock()
>> Third working alternative:
>>                  wlock = repo.wlock()
>>                  lock = repo.wlock()
>>
>> Basically I thought it would be good to use 1 of the 3 alternatives
>> throughout.
>>
>>> Why is this necessary? The kwrepo class does not override the wlock
>>> and lock methods, so I don't see why you would explicitly call the
>>> superclass' methods?
>> I'm not sure if this is necessary either. My layman's understanding
>> is: it doesn't break, it works, fine ;-) However, this attitude is
>> difficult to reconcile with my desire for consistency. And after
>> Matt's reply I /believe/ calling the superclass method explicitly is
>> safer.
> 
> I think he was talking about a different situation. He talked about
> doing
> 
>   self.foo = self.bar

The point is

    self.func2 = self.func1

Taking a reference to a member function "func1" binds it to that specific
object (via a new object), so it can be called like any other (non-member)
function -- that is, without providing the object again when calling it.

If you assign *that* to another member variable of the same object, you
get a cycle, which needs to be broken up by the GC when cleaning up.

> which apparently creates a reference cycle between self and itself(!)...
> I don't know the finer details of Python reference counting, but it
> surpricing that such an assignment should be dangerous.
> 
> However, you're simply calling a superclass' method and assigning the
> result to a local variable. When the variable goes out of scope the
> refcount of the superclass should go down as expected.
> 
> In any case, I think I'll let Matt or someone else decide on these
> patches :-)