Skip to content

Commit ff9a65c

Browse files
authored
Merge pull request #9193 from elpaso/bugfix-21270-processing-algrunner-crash
Processing: fix crash in alg runner task with bad scripts
2 parents 9c888bb + 7acfefa commit ff9a65c

File tree

7 files changed

+141
-9
lines changed

7 files changed

+141
-9
lines changed

python/core/auto_generated/processing/qgsprocessingalgorithm.sip.in

+5-2
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ a pre-initialized copy of the algorithm.
6363

6464

6565

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

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

79+
Raises a QgsProcessingException if a new algorithm instance could not be created,
80+
e.g. if there is an issue with the subclass' createInstance() method.
81+
7982
.. seealso:: :py:func:`initAlgorithm`
8083
%End
8184

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

402405
protected:
403406

404-
virtual QgsProcessingAlgorithm *createInstance() const = 0 /Factory/;
407+
virtual QgsProcessingAlgorithm *createInstance() const = 0 /Factory,VirtualErrorHandler=processing_exception_handler/;
405408
%Docstring
406409
Creates a new instance of the algorithm class.
407410

python/plugins/processing/gui/AlgorithmDialog.py

+5-2
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,11 @@ def on_complete(ok, results):
263263
self.showLog()
264264

265265
task = QgsProcessingAlgRunnerTask(self.algorithm(), parameters, self.context, self.feedback)
266-
task.executed.connect(on_complete)
267-
self.setCurrentTask(task)
266+
if task.isCanceled():
267+
on_complete(False, {})
268+
else:
269+
task.executed.connect(on_complete)
270+
self.setCurrentTask(task)
268271
else:
269272
self.proxy_progress = QgsProxyProgressTask(QCoreApplication.translate("AlgorithmDialog", "Executing “{}”").format(self.algorithm().displayName()))
270273
QgsApplication.taskManager().addTask(self.proxy_progress)

src/core/processing/qgsprocessingalgorithm.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ QgsProcessingAlgorithm::~QgsProcessingAlgorithm()
4040
QgsProcessingAlgorithm *QgsProcessingAlgorithm::create( const QVariantMap &configuration ) const
4141
{
4242
std::unique_ptr< QgsProcessingAlgorithm > creation( createInstance() );
43+
if ( ! creation )
44+
throw QgsProcessingException( QObject::tr( "Error creating algorithm from createInstance()" ) );
4345
creation->setProvider( provider() );
4446
creation->initAlgorithm( configuration );
4547
return creation.release();

src/core/processing/qgsprocessingalgorithm.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,12 @@ class CORE_EXPORT QgsProcessingAlgorithm
122122
* algorithms in a model, allowing them to adjust their behavior at run time
123123
* according to some user configuration.
124124
*
125+
* Raises a QgsProcessingException if a new algorithm instance could not be created,
126+
* e.g. if there is an issue with the subclass' createInstance() method.
127+
*
125128
* \see initAlgorithm()
126129
*/
127-
QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const SIP_TRANSFERBACK;
130+
QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const SIP_THROW( QgsProcessingException ) SIP_TRANSFERBACK;
128131

129132
/**
130133
* Returns the algorithm name, used for identifying the algorithm. This string
@@ -409,7 +412,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
409412
*
410413
* This method should return a 'pristine' instance of the algorithm class.
411414
*/
412-
virtual QgsProcessingAlgorithm *createInstance() const = 0 SIP_FACTORY;
415+
virtual QgsProcessingAlgorithm *createInstance() const = 0 SIP_FACTORY SIP_VIRTUALERRORHANDLER( processing_exception_handler );
413416

414417
/**
415418
* Initializes the algorithm using the specified \a configuration.

src/core/processing/qgsprocessingalgrunnertask.cpp

+13-3
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,30 @@ QgsProcessingAlgRunnerTask::QgsProcessingAlgRunnerTask( const QgsProcessingAlgor
2727
, mParameters( parameters )
2828
, mContext( context )
2929
, mFeedback( feedback )
30-
, mAlgorithm( algorithm->create() )
3130
{
3231
if ( !mFeedback )
3332
{
3433
mOwnedFeedback.reset( new QgsProcessingFeedback() );
3534
mFeedback = mOwnedFeedback.get();
3635
}
37-
if ( !mAlgorithm->prepare( mParameters, context, mFeedback ) )
36+
try
37+
{
38+
mAlgorithm.reset( algorithm->create() );
39+
if ( !( mAlgorithm && mAlgorithm->prepare( mParameters, context, mFeedback ) ) )
40+
cancel();
41+
}
42+
catch ( QgsProcessingException &e )
43+
{
44+
QgsMessageLog::logMessage( e.what(), QObject::tr( "Processing" ), Qgis::Critical );
45+
mFeedback->reportError( e.what() );
3846
cancel();
47+
}
3948
}
4049

4150
void QgsProcessingAlgRunnerTask::cancel()
4251
{
43-
mFeedback->cancel();
52+
if ( mFeedback )
53+
mFeedback->cancel();
4454
QgsTask::cancel();
4555
}
4656

tests/src/python/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ ADD_PYTHON_TEST(PyQgsPointDisplacementRenderer test_qgspointdisplacementrenderer
154154
ADD_PYTHON_TEST(PyQgsPostgresDomain test_qgspostgresdomain.py)
155155
ADD_PYTHON_TEST(PyQgsProcessingRecentAlgorithmLog test_qgsprocessingrecentalgorithmslog.py)
156156
ADD_PYTHON_TEST(PyQgsProcessingInPlace test_qgsprocessinginplace.py)
157+
ADD_PYTHON_TEST(PyQgsProcessingAlgRunner test_qgsprocessingalgrunner.py)
157158
ADD_PYTHON_TEST(PyQgsProcessingAlgDecorator test_processing_alg_decorator.py)
158159
ADD_PYTHON_TEST(PyQgsProjectionSelectionWidgets test_qgsprojectionselectionwidgets.py)
159160
ADD_PYTHON_TEST(PyQgsProjectMetadata test_qgsprojectmetadata.py)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# -*- coding: utf-8 -*-
2+
"""QGIS Unit tests for Processing algorithm runner(s).
3+
4+
.. note:: This program is free software; you can redistribute it and/or modify
5+
it under the terms of the GNU General Public License as published by
6+
the Free Software Foundation; either version 2 of the License, or
7+
(at your option) any later version.
8+
"""
9+
__author__ = 'Alessandro Pasotti'
10+
__date__ = '2019-02'
11+
__copyright__ = 'Copyright 2019, The QGIS Project'
12+
# This will get replaced with a git SHA1 when you do a git archive
13+
__revision__ = '$Format:%H$'
14+
15+
import re
16+
from qgis.PyQt.QtCore import QCoreApplication
17+
from qgis.testing import start_app, unittest
18+
from qgis.core import QgsProcessingAlgRunnerTask
19+
20+
from processing.core.Processing import Processing
21+
from processing.core.ProcessingConfig import ProcessingConfig
22+
from qgis.testing import start_app, unittest
23+
from qgis.analysis import QgsNativeAlgorithms
24+
from qgis.core import (
25+
QgsApplication,
26+
QgsSettings,
27+
QgsProcessingContext,
28+
QgsProcessingAlgRunnerTask,
29+
QgsProcessingAlgorithm,
30+
QgsProject,
31+
QgsProcessingFeedback,
32+
)
33+
34+
start_app()
35+
36+
37+
class ConsoleFeedBack(QgsProcessingFeedback):
38+
39+
_error = ''
40+
41+
def reportError(self, error, fatalError=False):
42+
self._error = error
43+
print(error)
44+
45+
46+
class CrashingProcessingAlgorithm(QgsProcessingAlgorithm):
47+
"""
48+
Wrong class in factory createInstance()
49+
"""
50+
51+
INPUT = 'INPUT'
52+
OUTPUT = 'OUTPUT'
53+
54+
def tr(self, string):
55+
return QCoreApplication.translate('Processing', string)
56+
57+
def createInstance(self):
58+
"""Wrong!"""
59+
return ExampleProcessingAlgorithm()
60+
61+
def name(self):
62+
return 'mycrashingscript'
63+
64+
def displayName(self):
65+
return self.tr('My Crashing Script')
66+
67+
def group(self):
68+
return self.tr('Example scripts')
69+
70+
def groupId(self):
71+
return 'examplescripts'
72+
73+
def shortHelpString(self):
74+
return self.tr("Example algorithm short description")
75+
76+
def initAlgorithm(self, config=None):
77+
pass
78+
79+
def processAlgorithm(self, parameters, context, feedback):
80+
return {self.OUTPUT: 'an_id'}
81+
82+
83+
class TestQgsProcessingAlgRunner(unittest.TestCase):
84+
85+
@classmethod
86+
def setUpClass(cls):
87+
"""Run before all tests"""
88+
QCoreApplication.setOrganizationName("QGIS_Test")
89+
QCoreApplication.setOrganizationDomain(
90+
"QGIS_TestPyQgsProcessingInPlace.com")
91+
QCoreApplication.setApplicationName("QGIS_TestPyQgsProcessingInPlace")
92+
QgsSettings().clear()
93+
Processing.initialize()
94+
QgsApplication.processingRegistry().addProvider(QgsNativeAlgorithms())
95+
cls.registry = QgsApplication.instance().processingRegistry()
96+
97+
def test_bad_script_dont_crash(self): # spellok
98+
"""Test regression #21270 (segfault)"""
99+
100+
context = QgsProcessingContext()
101+
context.setProject(QgsProject.instance())
102+
feedback = ConsoleFeedBack()
103+
104+
task = QgsProcessingAlgRunnerTask(CrashingProcessingAlgorithm(), {}, context=context, feedback=feedback)
105+
self.assertTrue(task.isCanceled())
106+
self.assertIn('name \'ExampleProcessingAlgorithm\' is not defined', feedback._error)
107+
108+
109+
if __name__ == '__main__':
110+
unittest.main()

0 commit comments

Comments
 (0)