[PyQt] SIP: memory leak in mapped type?

[Phil Thompson]

On Friday 27 July 2007 4:30 pm, Giovanni Bajo wrote:
> On 7/27/2007 3:19 PM, Phil Thompson wrote:
> >>          // Get the Python wrapper for the Type instance, creating a new
> >>          // one if necessary, and handle any ownership transfer.
> >>          if ((pobj = sipConvertFromInstance(cpp, sipClass_TYPE,
> >> sipTransferObj)) == NULL)
> >
> > Using sipConvertNewFromInstance() is preferable.
>
> I suggest you updated the examples in the SIP documentation, then.

No, the code is different. You are making a copy of the element.

> >>          {
> >>              // There was an error so garbage collect the Python list.
> >>              Py_DECREF(l);
> >
> > delete cpp?
>
> Right.
>
> >> But then, I was wondering about the semantic of sipTransferObj. The
> >> documentation isn't very clear about its usage. In fact, it's not clear
> >> in this case whether it should apply to the whole vectors or to the
> >> individual objects in the vector.
> >
> > What it applies to is entirely up to you. In PyQt it always applies to
> > the individual elements.
>
> Then, I haven't understood what "sipTransferObj" is or mean. Would you
> please explain it again from scratch? I have totally lost track of its
> meaning. Who sets its value? On which conditions? What it should apply to?

SIP maintains a tree of Python objects that reflects the tree of C++ 
instances. The tree's aren't identical because there may be more going on in 
the C++ library than Python is aware of. However, if it is working properly, 
the relative positions of Python objects and the corresponding C++ instances 
will be the same.

A node in the Python tree keeps a reference to its children. The purpose of 
the tree is to have those extra references and support the cyclic garbage 
collector.

The tree is manipulated using the various transfer annotations which control 
the value of sipTransferObj. Typically sipTransferObj is the parent Python 
object of the object you are about to create when wrapping a C++ instance. 
Normally you just pass it to another SIP function which will do the right 
thing.

> >> Does the fix make sense?
> >
> > No.
> >
> >> And if so, why it isn't required in the SIP
> >> documentation example? And finally, with the above patch, the value of
> >> sipTransferObj is totally ignored by the code: how can that be right?
> >
> > For it to have any effect you must have used one of the Transfer
> > annotations on an argument or result of type std::vector<>. That will
> > cause extra element references to be taken - maybe that's not
> > appropriate?
>
> I'm not sure what you mean.
>
> Please, have a look at the attached testcase. It shows two different (or
> related?) memory leaks:
>
> - bug.h / bug.cpp is the source code, with two functions with identical
> semantic but different signature.
> - bug.sip is the trivial wrapper for both functions.
> - stl.sip is the file with the mapped types for std::vector [notice that
> most of it should be builtin in SIP in my humble opinion, but this is a
> discussion for another day].
> - bug.py triggers the memory leak. You can choose which function to test
> in it. Both of them cause the leak.
>
> To compile, "setup.py build_ext --inplace".

There is a SIP bug (fixed in tonight's snapshot) which affects mapped types 
with the /Out/ annotation - but it has nothing to do with object ownership.

Phil