Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #9193 from elpaso/bugfix-21270-processing-algrunne…
…r-crash

Processing: fix crash in alg runner task with bad scripts

Cherry-picked from master commit ff9a65c
  • Loading branch information
elpaso committed Feb 20, 2019
1 parent 2a04ba5 commit 28650e7
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 9 deletions.
Expand Up @@ -63,7 +63,7 @@ a pre-initialized copy of the algorithm.



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

Expand All @@ -76,6 +76,9 @@ and outputs according to this configuration. This is generally used only for
algorithms in a model, allowing them to adjust their behavior at run time
according to some user configuration.

Raises a QgsProcessingException if a new algorithm instance could not be created,
e.g. if there is an issue with the subclass' createInstance() method.

.. seealso:: :py:func:`initAlgorithm`
%End

Expand Down Expand Up @@ -401,7 +404,7 @@ Associates this algorithm with its provider. No transfer of ownership is involve

protected:

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

Expand Down
7 changes: 5 additions & 2 deletions python/plugins/processing/gui/AlgorithmDialog.py
Expand Up @@ -263,8 +263,11 @@ def on_complete(ok, results):
self.showLog()

task = QgsProcessingAlgRunnerTask(self.algorithm(), parameters, self.context, self.feedback)
task.executed.connect(on_complete)
self.setCurrentTask(task)
if task.isCanceled():
on_complete(False, {})
else:
task.executed.connect(on_complete)
self.setCurrentTask(task)
else:
self.proxy_progress = QgsProxyProgressTask(QCoreApplication.translate("AlgorithmDialog", "Executing “{}”").format(self.algorithm().displayName()))
QgsApplication.taskManager().addTask(self.proxy_progress)
Expand Down
2 changes: 2 additions & 0 deletions src/core/processing/qgsprocessingalgorithm.cpp
Expand Up @@ -37,6 +37,8 @@ QgsProcessingAlgorithm::~QgsProcessingAlgorithm()
QgsProcessingAlgorithm *QgsProcessingAlgorithm::create( const QVariantMap &configuration ) const
{
std::unique_ptr< QgsProcessingAlgorithm > creation( createInstance() );
if ( ! creation )
throw QgsProcessingException( QObject::tr( "Error creating algorithm from createInstance()" ) );
creation->setProvider( provider() );
creation->initAlgorithm( configuration );
return creation.release();
Expand Down
7 changes: 5 additions & 2 deletions src/core/processing/qgsprocessingalgorithm.h
Expand Up @@ -121,9 +121,12 @@ class CORE_EXPORT QgsProcessingAlgorithm
* algorithms in a model, allowing them to adjust their behavior at run time
* according to some user configuration.
*
* Raises a QgsProcessingException if a new algorithm instance could not be created,
* e.g. if there is an issue with the subclass' createInstance() method.
*
* \see initAlgorithm()
*/
QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const SIP_TRANSFERBACK;
QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const SIP_THROW( QgsProcessingException ) SIP_TRANSFERBACK;

/**
* Returns the algorithm name, used for identifying the algorithm. This string
Expand Down Expand Up @@ -408,7 +411,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
*
* This method should return a 'pristine' instance of the algorithm class.
*/
virtual QgsProcessingAlgorithm *createInstance() const = 0 SIP_FACTORY;
virtual QgsProcessingAlgorithm *createInstance() const = 0 SIP_FACTORY SIP_VIRTUALERRORHANDLER( processing_exception_handler );

/**
* Initializes the algorithm using the specified \a configuration.
Expand Down
16 changes: 13 additions & 3 deletions src/core/processing/qgsprocessingalgrunnertask.cpp
Expand Up @@ -27,20 +27,30 @@ QgsProcessingAlgRunnerTask::QgsProcessingAlgRunnerTask( const QgsProcessingAlgor
, mParameters( parameters )
, mContext( context )
, mFeedback( feedback )
, mAlgorithm( algorithm->create() )
{
if ( !mFeedback )
{
mOwnedFeedback.reset( new QgsProcessingFeedback() );
mFeedback = mOwnedFeedback.get();
}
if ( !mAlgorithm->prepare( mParameters, context, mFeedback ) )
try
{
mAlgorithm.reset( algorithm->create() );
if ( !( mAlgorithm && mAlgorithm->prepare( mParameters, context, mFeedback ) ) )
cancel();
}
catch ( QgsProcessingException &e )
{
QgsMessageLog::logMessage( e.what(), QObject::tr( "Processing" ), Qgis::Critical );
mFeedback->reportError( e.what() );
cancel();
}
}

void QgsProcessingAlgRunnerTask::cancel()
{
mFeedback->cancel();
if ( mFeedback )
mFeedback->cancel();
QgsTask::cancel();
}

Expand Down
2 changes: 2 additions & 0 deletions tests/src/python/CMakeLists.txt
Expand Up @@ -146,6 +146,8 @@ ADD_PYTHON_TEST(PyQgsPointDisplacementRenderer test_qgspointdisplacementrenderer
ADD_PYTHON_TEST(PyQgsPostgresDomain test_qgspostgresdomain.py)
ADD_PYTHON_TEST(PyQgsProcessingRecentAlgorithmLog test_qgsprocessingrecentalgorithmslog.py)
ADD_PYTHON_TEST(PyQgsProcessingInPlace test_qgsprocessinginplace.py)
ADD_PYTHON_TEST(PyQgsProcessingAlgRunner test_qgsprocessingalgrunner.py)
ADD_PYTHON_TEST(PyQgsProcessingAlgDecorator test_processing_alg_decorator.py)
ADD_PYTHON_TEST(PyQgsProjectionSelectionWidgets test_qgsprojectionselectionwidgets.py)
ADD_PYTHON_TEST(PyQgsProjectMetadata test_qgsprojectmetadata.py)
ADD_PYTHON_TEST(PyQgsRange test_qgsrange.py)
Expand Down
110 changes: 110 additions & 0 deletions tests/src/python/test_qgsprocessingalgrunner.py
@@ -0,0 +1,110 @@
# -*- coding: utf-8 -*-
"""QGIS Unit tests for Processing algorithm runner(s).
.. note:: This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
"""
__author__ = 'Alessandro Pasotti'
__date__ = '2019-02'
__copyright__ = 'Copyright 2019, The QGIS Project'
# This will get replaced with a git SHA1 when you do a git archive
__revision__ = '$Format:%H$'

import re
from qgis.PyQt.QtCore import QCoreApplication
from qgis.testing import start_app, unittest
from qgis.core import QgsProcessingAlgRunnerTask

from processing.core.Processing import Processing
from processing.core.ProcessingConfig import ProcessingConfig
from qgis.testing import start_app, unittest
from qgis.analysis import QgsNativeAlgorithms
from qgis.core import (
QgsApplication,
QgsSettings,
QgsProcessingContext,
QgsProcessingAlgRunnerTask,
QgsProcessingAlgorithm,
QgsProject,
QgsProcessingFeedback,
)

start_app()


class ConsoleFeedBack(QgsProcessingFeedback):

_error = ''

def reportError(self, error, fatalError=False):
self._error = error
print(error)


class CrashingProcessingAlgorithm(QgsProcessingAlgorithm):
"""
Wrong class in factory createInstance()
"""

INPUT = 'INPUT'
OUTPUT = 'OUTPUT'

def tr(self, string):
return QCoreApplication.translate('Processing', string)

def createInstance(self):
"""Wrong!"""
return ExampleProcessingAlgorithm()

def name(self):
return 'mycrashingscript'

def displayName(self):
return self.tr('My Crashing Script')

def group(self):
return self.tr('Example scripts')

def groupId(self):
return 'examplescripts'

def shortHelpString(self):
return self.tr("Example algorithm short description")

def initAlgorithm(self, config=None):
pass

def processAlgorithm(self, parameters, context, feedback):
return {self.OUTPUT: 'an_id'}


class TestQgsProcessingAlgRunner(unittest.TestCase):

@classmethod
def setUpClass(cls):
"""Run before all tests"""
QCoreApplication.setOrganizationName("QGIS_Test")
QCoreApplication.setOrganizationDomain(
"QGIS_TestPyQgsProcessingInPlace.com")
QCoreApplication.setApplicationName("QGIS_TestPyQgsProcessingInPlace")
QgsSettings().clear()
Processing.initialize()
QgsApplication.processingRegistry().addProvider(QgsNativeAlgorithms())
cls.registry = QgsApplication.instance().processingRegistry()

def test_bad_script_dont_crash(self): # spellok
"""Test regression #21270 (segfault)"""

context = QgsProcessingContext()
context.setProject(QgsProject.instance())
feedback = ConsoleFeedBack()

task = QgsProcessingAlgRunnerTask(CrashingProcessingAlgorithm(), {}, context=context, feedback=feedback)
self.assertTrue(task.isCanceled())
self.assertIn('name \'ExampleProcessingAlgorithm\' is not defined', feedback._error)


if __name__ == '__main__':
unittest.main()

0 comments on commit 28650e7

Please sign in to comment.