[PyQt] Re: SIP bug with object lifetime

[Giovanni Bajo]

On 5/14/2007 11:01 AM, Phil Thompson wrote:

>>>>>> =========================================
>>>>>> from qt import *
>>>>>> import weakref
>>>>>>
>>>>>> app = QApplication([])
>>>>>>
>>>>>> ql = QListView(None)
>>>>>> viewport = ql.viewport()
>>>>>>
>>>>>> o = QObject(viewport)
>>>>>> o.xxx = viewport  # bug-trigger!
>>>>>>
>>>>>> destroyed = []
>>>>>> def cb(wr):
>>>>>>      destroyed.append(1)
>>>>>> wr = weakref.ref(o, cb)
>>>>>>
>>>>>> del o
>>>>>> del viewport
>>>>>> assert not destroyed, "object destroyed too early #1!"
>>>>>>
>>>>>> import gc
>>>>>> gc.collect()
>>>>>> assert not destroyed, "object destroyed too early #2!"
>>>>>>
>>>>>> del ql
>>>>>> import gc
>>>>>> gc.collect()
>>>>>> assert destroyed, "object never destroyed!"
>>>>>> =========================================
>>>>>> Traceback (most recent call last):
>>>>>>    File "pyqtbug19.py", line 25, in ?
>>>>>>      assert not destroyed, "object destroyed too early #2!"
>>>>>> AssertionError: object destroyed too early #2!
>>>>>>
>>>>>>
>>>>>> This happens with latest PyQt and SIP official releases (3.17.1 and
>>>>>> 4.6). The line that seems to trigger the bug is the one marked with a
>>>>>> comment.
>>>>> This behaves as I would expect - ie. it's a missing feature rather than
>>>>> a bug.
>>>>>
>>>>> Although ql is the parent of viewport, Python doesn't know that and
>>>>> there is no hidden extra reference to viewport to keep it alive when
>>>>> collect() is run.
>>>> But the problem here is that "o" is collected. o is a QObject whose
>>>> lifetime should be transferred to C++ (given that it has a non-NULL
>>>> parent, right?).
>>> But that parent is owned by Python. When viewpoint (Python) goes,
>>> viewpoint (C++) goes, which takes o (C++), which takes o (Python).
>> Are you saying that "viewport" is owned by Python?
> 
> I was but I was just in the process of replying to myself to say I was talking 
> rubbish.
> 
> Ownership these days covers a couple of things that are actually applied 
> separately. One thing is the responsibility to call the C++ dtor, the other 
> thing is the associations maintained for the cyclic garbage collector.
> 
> When the result of viewport() is wrapped (assuming this is the first call) 
> then C++ has the responsibility for calling the dtor. However as far as 
> Python is concerned there is no gc association with any other Python object.
> 
> "Association" is a parent/child relationship and the parent has an (internally 
> maintained) Python reference to the child.
> 
> When o is wrapped C++ is responsible for the dtor. A gc association is also 
> made with viewport, ie. the viewport Python object has a reference to the o 
> Python object. Obviously o is given a reference to viewport via the xxx 
> attribute, so creating the cycle.
> 
> When o and viewport are del'ed and collect() is run the cycle is broken and 
> the o and viewport Python objects garbage collected. The o and viewport C++ 
> instances are left untouched.

OK, this clears up everything but why both objects manage to survive 
even to a gc.collect() call if the "xxx" attribute does not exist.

> If the new /Transfer/ function annotation were to be applied to viewport() 
> then a gc association would be established between viewport and ql with the 
> ql Python object having a reference to the viewport Python object. This would 
> prevent viewport (and also o) from being garbage collected until ql was.

What about queryList()/findChildren()? In my original code, the 
C++-allocated instance was accessed through a queryList() call. It's 
basically the same (getting a reference to a C++ instance never seen 
before by Python), but I guess the annotation won't be enough...

I wonder: can't you just apply it automatically to all cases where you 
create a wrapper for an already existing C++ object that was never 
wrapped before? How can't that be wrong?

>> (PS: what about a debugging function in sip which tells who owns who?
>> sip.owner(foo) which returns a sip.voidptr() to the owner if it's C++, or
>> None if it's owned by Python. It would make debugging of situations like
>> this a little easier... using weakref/SIGNAL(destroyed) can bring other
>> bugs into the table and makes things more confusing)
> 
> Would you want to use the value? Or would something like sip.debug(foo) that 
> displayed lots of stuff to stdout be better?

I was thinking it just for debugging purposes, so sip.debug(foo) should 
be fine.
-- 
Giovanni Bajo