[PyQt] Custom exception when transferring "owned" object

[Phil Thompson]

On 29 Mar 2019, at 6:32 am, Nyall Dawson <nyall.dawson at gmail.com> wrote:
> 
> Hi list,
> 
> I'm trying to find a way to raise a custom exception if someone
> attempts to call a method which takes ownership with an object which
> is already owned elsewhere (instead of crashing at some future
> stage!).
> 
> Here's what I've got so far:
> 
> .h:
> 
> bool addGeometry( QgsAbstractGeometry *g );
> 
> .sip:
> 
> %MethodCode
>    PyObject *obj = sipConvertFromType( a0, sipType_QgsAbstractGeometry, NULL );
>    if ( !sipIsPyOwned( ( sipSimpleWrapper * )obj ) )
>    {
>      PyErr_SetString( sipException_OwnershipException, "Geometry is
> already owned by another c++ object. Use .clone() to add a deep copy
> of the geometry to this multipoint." );
>      sipIsErr = 1;
>      Py_DECREF( obj );
>    }
>    else
>    {
>      bool res = sipCpp->addGeometry( a0 );
>      if ( res )
>      {
>        PyObject *owner = sipConvertFromType( sipCpp,
> sipType_QgsAbstractGeometry, NULL );
>        sipTransferTo( obj,  owner );
>      }
>      return PyBool_FromLong( res );
>    }
> %End
> 
> It works ok about 50% of the time, the other 50% it crashes. I suspect
> my methodcode isn't correct transferring ownership and the argument
> c++ object is getting deleted when its Python wrapper goes out of
> scope.
> 
> Can anyone see what I'm missing here?

sipConvertFromType() returns a new reference which you aren't dealing with properly.

You don't need the first call to sipConvertFromType(). Use /GetWrapper/ instead.

sipIsPyOwned() is an undocumented internal function and may be removed at any time.

Personally I think trying to add protection in the bindings layer to cover weaknesses in the (current) *implementation* of the C++ API is a mistake (and a never ending task).

Phil