sharpetronics/QGIS
1
0
Fork 0
mirror of https://github.com/qgis/QGIS.git synced 2025-04-15 00:04:00 -04:00
Code Issues Packages Projects Releases Wiki Activity

Don't crash when a null task is added to task manager

Browse Source
This commit is contained in:
Nyall Dawson 2018-05-25 15:51:14 +10:00
parent 595ecceb88
commit 43928d8c9c
4 changed files with 30 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
python/core/auto_generated/qgstaskmanager.sip.in
View File

@ -331,7 +331,7 @@ the task. The priority argument can be used to control the run queue's
order of execution, with larger numbers order of execution, with larger numbers
taking precedence over lower priority numbers. taking precedence over lower priority numbers.
:return: unique task ID :return: unique task ID, or 0 if task could not be added
%End %End
long addTask( const TaskDefinition &task /Transfer/, int priority = 0 ); long addTask( const TaskDefinition &task /Transfer/, int priority = 0 );
@ -342,7 +342,7 @@ manager will be responsible for starting the task. The priority argument can
be used to control the run queue's order of execution, with larger numbers be used to control the run queue's order of execution, with larger numbers
taking precedence over lower priority numbers. taking precedence over lower priority numbers.
:return: unique task ID :return: unique task ID, or 0 if task could not be added
%End %End
QgsTask *task( long id ) const; QgsTask *task( long id ) const;

3
src/core/qgstaskmanager.cpp
View File

@ -390,6 +390,9 @@ long QgsTaskManager::addTask( const QgsTaskManager::TaskDefinition &definition,
long QgsTaskManager::addTaskPrivate( QgsTask *task, QgsTaskList dependencies, bool isSubTask, int priority ) long QgsTaskManager::addTaskPrivate( QgsTask *task, QgsTaskList dependencies, bool isSubTask, int priority )
{ {
if ( !task )
return 0;
long taskId = mNextTaskId++; long taskId = mNextTaskId++;
mTaskMutex->lock(); mTaskMutex->lock();

6
src/core/qgstaskmanager.h
View File

@ -405,7 +405,7 @@ class CORE_EXPORT QgsTaskManager : public QObject
* the task. The priority argument can be used to control the run queue's * the task. The priority argument can be used to control the run queue's
* order of execution, with larger numbers * order of execution, with larger numbers
* taking precedence over lower priority numbers. * taking precedence over lower priority numbers.
* \returns unique task ID * \returns unique task ID, or 0 if task could not be added
*/ */
long addTask( QgsTask *task SIP_TRANSFER, int priority = 0 ); long addTask( QgsTask *task SIP_TRANSFER, int priority = 0 );
@ -415,7 +415,7 @@ class CORE_EXPORT QgsTaskManager : public QObject
* manager will be responsible for starting the task. The priority argument can * manager will be responsible for starting the task. The priority argument can
* be used to control the run queue's order of execution, with larger numbers * be used to control the run queue's order of execution, with larger numbers
* taking precedence over lower priority numbers. * taking precedence over lower priority numbers.
* \returns unique task ID * \returns unique task ID, or 0 if task could not be added
*/ */
long addTask( const TaskDefinition &task SIP_TRANSFER, int priority = 0 ); long addTask( const TaskDefinition &task SIP_TRANSFER, int priority = 0 );
@ -573,7 +573,7 @@ class CORE_EXPORT QgsTaskManager : public QObject
QMap< long, QgsWeakMapLayerPointerList > mLayerDependencies; QMap< long, QgsWeakMapLayerPointerList > mLayerDependencies;
//! Tracks the next unique task ID //! Tracks the next unique task ID
long mNextTaskId = 0; long mNextTaskId = 1;
//! List of active (queued or running) tasks. Includes subtasks. //! List of active (queued or running) tasks. Includes subtasks.
QSet< QgsTask * > mActiveTasks; QSet< QgsTask * > mActiveTasks;

41
tests/src/core/testqgstaskmanager.cpp
View File

@ -329,14 +329,17 @@ void TestQgsTaskManager::addTask()
QSignalSpy spy( &manager, &QgsTaskManager::taskAdded ); QSignalSpy spy( &manager, &QgsTaskManager::taskAdded );
// null task
QVERIFY( !manager.addTask( nullptr ) );
//add a task //add a task
CancelableTask *task = new CancelableTask(); CancelableTask *task = new CancelableTask();
long id = manager.addTask( task ); long id = manager.addTask( task );
QCOMPARE( id, 0L ); QCOMPARE( id, 1L );
QCOMPARE( manager.tasks().count(), 1 ); QCOMPARE( manager.tasks().count(), 1 );
QCOMPARE( manager.count(), 1 ); QCOMPARE( manager.count(), 1 );
QCOMPARE( spy.count(), 1 ); QCOMPARE( spy.count(), 1 );
QCOMPARE( spy.last().at( 0 ).toLongLong(), 0LL ); QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
while ( !task->isActive() ) while ( !task->isActive() )
{ {
QCoreApplication::processEvents(); QCoreApplication::processEvents();
@ -346,18 +349,18 @@ void TestQgsTaskManager::addTask()
QCOMPARE( task->status(), QgsTask::Running ); QCOMPARE( task->status(), QgsTask::Running );
//retrieve task //retrieve task
QCOMPARE( manager.task( 0L ), task ); QCOMPARE( manager.task( 1L ), task );
QCOMPARE( manager.tasks().at( 0 ), task ); QCOMPARE( manager.tasks().at( 0 ), task );
//add a second task //add a second task
CancelableTask *task2 = new CancelableTask(); CancelableTask *task2 = new CancelableTask();
id = manager.addTask( task2 ); id = manager.addTask( task2 );
QCOMPARE( id, 1L ); QCOMPARE( id, 2L );
QCOMPARE( manager.tasks().count(), 2 ); QCOMPARE( manager.tasks().count(), 2 );
QCOMPARE( manager.count(), 2 ); QCOMPARE( manager.count(), 2 );
QCOMPARE( manager.task( 0L ), task ); QCOMPARE( manager.task( 1L ), task );
QVERIFY( manager.tasks().contains( task ) ); QVERIFY( manager.tasks().contains( task ) );
QCOMPARE( manager.task( 1L ), task2 ); QCOMPARE( manager.task( 2L ), task2 );
QVERIFY( manager.tasks().contains( task2 ) ); QVERIFY( manager.tasks().contains( task2 ) );
while ( !task2->isActive() ) while ( !task2->isActive() )
{ {
@ -368,7 +371,7 @@ void TestQgsTaskManager::addTask()
QCOMPARE( task2->status(), QgsTask::Running ); QCOMPARE( task2->status(), QgsTask::Running );
QCOMPARE( spy.count(), 2 ); QCOMPARE( spy.count(), 2 );
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL ); QCOMPARE( spy.last().at( 0 ).toLongLong(), 2LL );
task->cancel(); task->cancel();
task2->cancel(); task2->cancel();
@ -649,8 +652,8 @@ void TestQgsTaskManager::taskId()
TestTask *task3 = new TestTask(); TestTask *task3 = new TestTask();
QCOMPARE( manager.taskId( nullptr ), -1L ); QCOMPARE( manager.taskId( nullptr ), -1L );
QCOMPARE( manager.taskId( task ), 0L ); QCOMPARE( manager.taskId( task ), 1L );
QCOMPARE( manager.taskId( task2 ), 1L ); QCOMPARE( manager.taskId( task2 ), 2L );
QCOMPARE( manager.taskId( task3 ), -1L ); QCOMPARE( manager.taskId( task3 ), -1L );
delete task3; delete task3;
@ -725,7 +728,7 @@ void TestQgsTaskManager::progressChanged()
QCOMPARE( task->progress(), 50.0 ); QCOMPARE( task->progress(), 50.0 );
QCOMPARE( spy.count(), 1 ); QCOMPARE( spy.count(), 1 );
QCOMPARE( spy.last().at( 0 ).toLongLong(), 0LL ); QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
QCOMPARE( spy.last().at( 1 ).toDouble(), 50.0 ); QCOMPARE( spy.last().at( 1 ).toDouble(), 50.0 );
//multiple running tasks, so finalTaskProgressChanged(double) should not be emitted //multiple running tasks, so finalTaskProgressChanged(double) should not be emitted
QCOMPARE( spy2.count(), 0 ); QCOMPARE( spy2.count(), 0 );
@ -733,7 +736,7 @@ void TestQgsTaskManager::progressChanged()
task2->emitProgressChanged( 75.0 ); task2->emitProgressChanged( 75.0 );
QCOMPARE( task2->progress(), 75.0 ); QCOMPARE( task2->progress(), 75.0 );
QCOMPARE( spy.count(), 2 ); QCOMPARE( spy.count(), 2 );
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL ); QCOMPARE( spy.last().at( 0 ).toLongLong(), 2LL );
QCOMPARE( spy.last().at( 1 ).toDouble(), 75.0 ); QCOMPARE( spy.last().at( 1 ).toDouble(), 75.0 );
QCOMPARE( spy2.count(), 0 ); QCOMPARE( spy2.count(), 0 );
@ -799,7 +802,7 @@ void TestQgsTaskManager::statusChanged()
flushEvents(); flushEvents();
QCOMPARE( spy.count(), 1 ); QCOMPARE( spy.count(), 1 );
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL ); QCOMPARE( spy.last().at( 0 ).toLongLong(), 2LL );
QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Running ); QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Running );
task->terminate(); task->terminate();
@ -809,7 +812,7 @@ void TestQgsTaskManager::statusChanged()
} }
flushEvents(); flushEvents();
QCOMPARE( spy.count(), 2 ); QCOMPARE( spy.count(), 2 );
QCOMPARE( spy.last().at( 0 ).toLongLong(), 0LL ); QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Terminated ); QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Terminated );
task2->finish(); task2->finish();
@ -819,7 +822,7 @@ void TestQgsTaskManager::statusChanged()
} }
flushEvents(); flushEvents();
QCOMPARE( spy.count(), 3 ); QCOMPARE( spy.count(), 3 );
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL ); QCOMPARE( spy.last().at( 0 ).toLongLong(), 2LL );
QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Complete ); QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Complete );
} }
@ -1126,7 +1129,7 @@ void TestQgsTaskManager::managerWithSubTasks()
QCOMPARE( spyProgress.count(), 0 ); QCOMPARE( spyProgress.count(), 0 );
subTask->emitProgressChanged( 50 ); subTask->emitProgressChanged( 50 );
QCOMPARE( spyProgress.count(), 1 ); QCOMPARE( spyProgress.count(), 1 );
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 0LL ); QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 1LL );
// subTask itself is 50% done, so with it's child task it's sitting at overall 25% done // subTask itself is 50% done, so with it's child task it's sitting at overall 25% done
// (one task 50%, one task not started) // (one task 50%, one task not started)
// parent task has two tasks (itself + subTask), and subTask is 25% done.... so parent // parent task has two tasks (itself + subTask), and subTask is 25% done.... so parent
@ -1135,11 +1138,11 @@ void TestQgsTaskManager::managerWithSubTasks()
subsubTask->emitProgressChanged( 100 ); subsubTask->emitProgressChanged( 100 );
QCOMPARE( spyProgress.count(), 2 ); QCOMPARE( spyProgress.count(), 2 );
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 0LL ); QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 1LL );
QCOMPARE( spyProgress.last().at( 1 ).toInt(), 38 ); QCOMPARE( spyProgress.last().at( 1 ).toInt(), 38 );
parent->emitProgressChanged( 50 ); parent->emitProgressChanged( 50 );
QCOMPARE( spyProgress.count(), 3 ); QCOMPARE( spyProgress.count(), 3 );
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 0LL ); QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 1LL );
QCOMPARE( spyProgress.last().at( 1 ).toInt(), 63 ); QCOMPARE( spyProgress.last().at( 1 ).toInt(), 63 );
//manager should not emit statusChanged signals for subtasks //manager should not emit statusChanged signals for subtasks
@ -1152,7 +1155,7 @@ void TestQgsTaskManager::managerWithSubTasks()
} }
flushEvents(); flushEvents();
QCOMPARE( statusSpy.count(), 1 ); QCOMPARE( statusSpy.count(), 1 );
QCOMPARE( statusSpy.last().at( 0 ).toLongLong(), 0LL ); QCOMPARE( statusSpy.last().at( 0 ).toLongLong(), 1LL );
QCOMPARE( static_cast< QgsTask::TaskStatus >( statusSpy.last().at( 1 ).toInt() ), QgsTask::Running ); QCOMPARE( static_cast< QgsTask::TaskStatus >( statusSpy.last().at( 1 ).toInt() ), QgsTask::Running );
subTask->finish(); subTask->finish();
@ -1170,7 +1173,7 @@ void TestQgsTaskManager::managerWithSubTasks()
} }
flushEvents(); flushEvents();
QCOMPARE( statusSpy.count(), 2 ); QCOMPARE( statusSpy.count(), 2 );
QCOMPARE( statusSpy.last().at( 0 ).toLongLong(), 0LL ); QCOMPARE( statusSpy.last().at( 0 ).toLongLong(), 1LL );
QCOMPARE( static_cast< QgsTask::TaskStatus >( statusSpy.last().at( 1 ).toInt() ), QgsTask::Complete ); QCOMPARE( static_cast< QgsTask::TaskStatus >( statusSpy.last().at( 1 ).toInt() ), QgsTask::Complete );