Re: Custom exception when transferring "owned" object

On Fri, 29 Mar 2019 at 19:59, Phil Thompson <[email protected]> wrote:
>
> On 29 Mar 2019, at 6:32 am, Nyall Dawson <[email protected]> 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.
>

(Reposting with the full context -- apologies for resurrecting such an
old thread!)

I noticed that there's now sipIsOwnedByPython in the public API. My
updated code looks like this:

    if ( !sipIsOwnedByPython( ( sipSimpleWrapper * )a0Wrapper ) )
    {
      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;
    }
    else
    {
      bool res = sipCpp->addGeometry( a0 );
      if ( res )
      {
        PyObject *owner = sipConvertFromType( sipCpp,
sipType_QgsAbstractGeometry, NULL );
        sipTransferTo( a0Wrapper,  owner );
        Py_DECREF( owner );
      }
      return PyBool_FromLong( res );

Does this approach look reasonable to you? Is it correctly using the
new sipIsOwnedByPython call?

Nyall
_______________________________________________
PyQt mailing list    [email protected]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt