[QGIS-Developer] Is it just me, or is sip totally broken?

[Martin Dobias]

Hi Nyall

I have seen that too with other QGIS registries, but I am not sure if
it is actually broken in SIP or if this should be simply handled in a
different way.

My understanding is that SIP for an object has the "c++ part" and
"python part". If we do not deal with subclasses, things seem to work
nicely: if you pass object of type X to function "myfunc(X* x
/Transfer/)" then the object gets ownership passed to C++, and then
when in python the object gets out of scope, its "python part" get
deleted because it is not necessary anymore and only the "c++ part"
keeps on living (and will be deleted in c++ code. The trouble is when
passing derived classes with /Transfer/ where the "python part" is
actually important, but it still gets deleted.

SIP also has /KeepReference/ annotation, but it's probably not that
useful here, because it is meant to keep reference to one python
object passed through API (if I had a function addCheck with
/KeepReference/ arg annotation then the first call to addCheck(a) will
keep the reference, but the second call addCheck(b) would drop
reference to "a"). The trouble is that if we want SIP hold reference
to the python object, we would need to give it extra information about
the object's life cycle (when is it safe to release the reference). So
we would probably need a /KeepReference/ variant with a matching
/ReleaseReference/ annotation for some other function - like this SIP
could track of multiple objects (which is the case of registries) and
handle their life cycle correctly. Maybe this registry pattern is not
used anywhere in Qt so SIP does not include support for it...

A simple "fix" is to just keep the reference to the object in the
python code. Not the nicest solution I know...

Cheers
Martin

On Thu, Jan 3, 2019 at 8:19 AM Nyall Dawson <nyall.dawson at gmail.com> wrote:
>
> Hi list,
>
> I'm at my wits end here, fighting with a sip issue I can't solve. And
> honestly, I'm starting to think maybe there's something completely
> broken in sip. (Or at least, in the version 4.19.7 provided by my
> distro).
>
> Here's what I'm currently hitting:
>
> The validity check registry added this week has a method "addCheck",
> which transfers ownership of the check to c++:
>
> void addCheck( QgsAbstractValidityCheck *check SIP_TRANSFER );
> https://github.com/qgis/QGIS/blob/master/src/core/validity/qgsvaliditycheckregistry.h#L65
>
> But if I make a Python subclass of QgsAbstractValidityCheck, and add
> it to the registry, it's immediately garbage collected by Python!
>
> My test code looks like this:
>
> class LayoutMapCrsCheck(QgsAbstractValidityCheck):
>
>     def __del__(self):
>         print('WTFFFFFFF!!!')
>
>     def id(self):
>         return 'map_crs_check'
>
>     def checkType(self):
>         return QgsAbstractValidityCheck.TypeLayoutCheck
>
>     def prepareCheck(self,context, feedback):
>         return True
>
>     def runCheck(self,context, feedback):
>         return []
>
> def add_check():
>     QgsApplication.validityCheckRegistry().addCheck(LayoutMapCrsCheck())
>
> add_check()
>
>
> So I create a Python subclass of the interface, and then add it to the
> registry. And boom - it's immediately deleted.
>
> If I make a global copy of the check, and then add to the registry,
> everything works -- the check isn't garbage collected.
>
> check_instance = LayoutMapCrsCheck()
> QgsApplication.validityCheckRegistry().addCheck(check_instance)
>
> But this leads me to believe that sip is broken somewhere, and is
> ignoring the /Transfer/ annotation.
>
> Indeed - I've also seen this in other classes. Specifically
> QgsTaskManager.addTask() -- Python tasks must be stored globally in
> order to avoid them being immediately deleted too.
>
> Can anyone else reproduce using my above code? Does anyone have any
> ideas what's happening here?
>
> Nyall
> _______________________________________________
> QGIS-Developer mailing list
> QGIS-Developer at lists.osgeo.org
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer