Skip to content
Permalink
Browse files

Fix "wrapped object has been deleted" errors in Processing

Ownership of Python subclass algorithm instances was getting
mangled due to passing through multiple functions with /Factory/
annotations.

As per Phil Thomson's advice on
https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
"
/Factory/ is used when the instance returned is guaranteed to be
new to Python. In this case it isn't because it has already been
seen when being returned by createInstance(). (However for a different
sub-class implemented in C++ then it would be the first time it was seen
by Python so the /Factory/ on create() would be correct.)

You might try using /TransferBack/ on create() instead - that might be
the best compromise.
"

Changing to /TransferBack/ indeed fixes the error for me.
  • Loading branch information
nyalldawson committed Jul 25, 2017
1 parent 1b9c5be commit 383422f069f9d067eef967abddf01e3da6a1167a
@@ -61,7 +61,8 @@ class QgsProcessingAlgorithm
virtual ~QgsProcessingAlgorithm();


QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const /Factory/;

QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const /TransferBack/;
%Docstring
Creates a copy of the algorithm, ready for execution.

@@ -360,19 +361,13 @@ class QgsProcessingAlgorithm

protected:

virtual QgsProcessingAlgorithm *createInstance() const = 0;
virtual QgsProcessingAlgorithm *createInstance() const = 0 /Factory/;
%Docstring
Creates a new instance of the algorithm class.

This method should return a 'pristine' instance of the algorithm class.
:rtype: QgsProcessingAlgorithm
%End
%VirtualCatcherCode
PyObject *resObj = sipCallMethod( 0, sipMethod, "" );
sipIsErr = !resObj || sipParseResult( 0, sipMethod, resObj, "H2", sipType_QgsProcessingAlgorithm, &sipRes ) < 0;
if ( !sipIsErr )
sipTransferTo( resObj, Py_None );
%End

virtual void initAlgorithm( const QVariantMap &configuration = QVariantMap() ) = 0;
%Docstring
@@ -89,7 +89,8 @@ class QgsProcessingRegistry : QObject
:rtype: QgsProcessingAlgorithm
%End

QgsProcessingAlgorithm *createAlgorithmById( const QString &id, const QVariantMap &configuration = QVariantMap() ) const /Factory/;

QgsProcessingAlgorithm *createAlgorithmById( const QString &id, const QVariantMap &configuration = QVariantMap() ) const /TransferBack/;
%Docstring
Creates a new instance of an algorithm by its ID. If no matching algorithm is found, a None
is returned. Callers take responsibility for deleting the returned object.
@@ -91,6 +91,21 @@ class CORE_EXPORT QgsProcessingAlgorithm
//! Algorithms cannot be copied- create() should be used instead
QgsProcessingAlgorithm &operator=( const QgsProcessingAlgorithm &other ) = delete;

/*
* IMPORTANT: While it seems like /Factory/ would be the correct annotation here, that's not
* the case.
* As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
*
* "
* /Factory/ is used when the instance returned is guaranteed to be new to Python.
* In this case it isn't because it has already been seen when being returned by createInstance()
* (However for a different sub-class implemented in C++ then it would be the first time it was seen
* by Python so the /Factory/ on create() would be correct.)
*
* You might try using /TransferBack/ on create() instead - that might be the best compromise.
* "
*/

/**
* Creates a copy of the algorithm, ready for execution.
*
@@ -105,7 +120,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
*
* \see initAlgorithm()
*/
QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const SIP_FACTORY;
QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const SIP_TRANSFERBACK;

/**
* Returns the algorithm name, used for identifying the algorithm. This string
@@ -357,15 +372,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
*
* This method should return a 'pristine' instance of the algorithm class.
*/
virtual QgsProcessingAlgorithm *createInstance() const = 0;
#ifdef SIP_RUN
SIP_VIRTUAL_CATCHER_CODE
PyObject *resObj = sipCallMethod( 0, sipMethod, "" );
sipIsErr = !resObj || sipParseResult( 0, sipMethod, resObj, "H2", sipType_QgsProcessingAlgorithm, &sipRes ) < 0;
if ( !sipIsErr )
sipTransferTo( resObj, Py_None );
SIP_END
#endif
virtual QgsProcessingAlgorithm *createInstance() const = 0 SIP_FACTORY;

/**
* Initializes the algorithm using the specified \a configuration.
@@ -100,6 +100,21 @@ class CORE_EXPORT QgsProcessingRegistry : public QObject
*/
const QgsProcessingAlgorithm *algorithmById( const QString &id ) const;

/*
* IMPORTANT: While it seems like /Factory/ would be the correct annotation here, that's not
* the case.
* As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
*
* "
* /Factory/ is used when the instance returned is guaranteed to be new to Python.
* In this case it isn't because it has already been seen when being returned by QgsProcessingAlgorithm::createInstance()
* (However for a different sub-class implemented in C++ then it would be the first time it was seen
* by Python so the /Factory/ on create() would be correct.)
*
* You might try using /TransferBack/ on create() instead - that might be the best compromise.
* "
*/

/**
* Creates a new instance of an algorithm by its ID. If no matching algorithm is found, a nullptr
* is returned. Callers take responsibility for deleting the returned object.
@@ -113,7 +128,7 @@ class CORE_EXPORT QgsProcessingRegistry : public QObject
* \see algorithms()
* \see algorithmById()
*/
QgsProcessingAlgorithm *createAlgorithmById( const QString &id, const QVariantMap &configuration = QVariantMap() ) const SIP_FACTORY;
QgsProcessingAlgorithm *createAlgorithmById( const QString &id, const QVariantMap &configuration = QVariantMap() ) const SIP_TRANSFERBACK;

signals:

0 comments on commit 383422f

Please sign in to comment.
You can’t perform that action at this time.