Skip to content

Commit 43928d8

Browse files
nyalldawsonnyalldawson
committedMay 25, 2018
Don't crash when a null task is added to task manager
·
ltr-3_40final-3_2_0
1 parent 595ecce commit 43928d8

File tree

4 files changed

+30
-24
lines changed

4 files changed

+30
-24
lines changed
 

‎python/core/auto_generated/qgstaskmanager.sip.in

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ the task. The priority argument can be used to control the run queue's
331331
order of execution, with larger numbers
332332
taking precedence over lower priority numbers.
333333

334-
:return: unique task ID
334+
:return: unique task ID, or 0 if task could not be added
335335
%End
336336

337337
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
342342
be used to control the run queue's order of execution, with larger numbers
343343
taking precedence over lower priority numbers.
344344

345-
:return: unique task ID
345+
:return: unique task ID, or 0 if task could not be added
346346
%End
347347

348348
QgsTask *task( long id ) const;

‎src/core/qgstaskmanager.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,9 @@ long QgsTaskManager::addTask( const QgsTaskManager::TaskDefinition &definition,
390390

391391
long QgsTaskManager::addTaskPrivate( QgsTask *task, QgsTaskList dependencies, bool isSubTask, int priority )
392392
{
393+
if ( !task )
394+
return 0;
395+
393396
long taskId = mNextTaskId++;
394397

395398
mTaskMutex->lock();

‎src/core/qgstaskmanager.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ class CORE_EXPORT QgsTaskManager : public QObject
405405
* the task. The priority argument can be used to control the run queue's
406406
* order of execution, with larger numbers
407407
* taking precedence over lower priority numbers.
408-
* \returns unique task ID
408+
* \returns unique task ID, or 0 if task could not be added
409409
*/
410410
long addTask( QgsTask *task SIP_TRANSFER, int priority = 0 );
411411

@@ -415,7 +415,7 @@ class CORE_EXPORT QgsTaskManager : public QObject
415415
* manager will be responsible for starting the task. The priority argument can
416416
* be used to control the run queue's order of execution, with larger numbers
417417
* taking precedence over lower priority numbers.
418-
* \returns unique task ID
418+
* \returns unique task ID, or 0 if task could not be added
419419
*/
420420
long addTask( const TaskDefinition &task SIP_TRANSFER, int priority = 0 );
421421

@@ -573,7 +573,7 @@ class CORE_EXPORT QgsTaskManager : public QObject
573573
QMap< long, QgsWeakMapLayerPointerList > mLayerDependencies;
574574

575575
//! Tracks the next unique task ID
576-
long mNextTaskId = 0;
576+
long mNextTaskId = 1;
577577

578578
//! List of active (queued or running) tasks. Includes subtasks.
579579
QSet< QgsTask * > mActiveTasks;

‎tests/src/core/testqgstaskmanager.cpp

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -329,14 +329,17 @@ void TestQgsTaskManager::addTask()
329329

330330
QSignalSpy spy( &manager, &QgsTaskManager::taskAdded );
331331

332+
// null task
333+
QVERIFY( !manager.addTask( nullptr ) );
334+
332335
//add a task
333336
CancelableTask *task = new CancelableTask();
334337
long id = manager.addTask( task );
335-
QCOMPARE( id, 0L );
338+
QCOMPARE( id, 1L );
336339
QCOMPARE( manager.tasks().count(), 1 );
337340
QCOMPARE( manager.count(), 1 );
338341
QCOMPARE( spy.count(), 1 );
339-
QCOMPARE( spy.last().at( 0 ).toLongLong(), 0LL );
342+
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
340343
while ( !task->isActive() )
341344
{
342345
QCoreApplication::processEvents();
@@ -346,18 +349,18 @@ void TestQgsTaskManager::addTask()
346349
QCOMPARE( task->status(), QgsTask::Running );
347350

348351
//retrieve task
349-
QCOMPARE( manager.task( 0L ), task );
352+
QCOMPARE( manager.task( 1L ), task );
350353
QCOMPARE( manager.tasks().at( 0 ), task );
351354

352355
//add a second task
353356
CancelableTask *task2 = new CancelableTask();
354357
id = manager.addTask( task2 );
355-
QCOMPARE( id, 1L );
358+
QCOMPARE( id, 2L );
356359
QCOMPARE( manager.tasks().count(), 2 );
357360
QCOMPARE( manager.count(), 2 );
358-
QCOMPARE( manager.task( 0L ), task );
361+
QCOMPARE( manager.task( 1L ), task );
359362
QVERIFY( manager.tasks().contains( task ) );
360-
QCOMPARE( manager.task( 1L ), task2 );
363+
QCOMPARE( manager.task( 2L ), task2 );
361364
QVERIFY( manager.tasks().contains( task2 ) );
362365
while ( !task2->isActive() )
363366
{
@@ -368,7 +371,7 @@ void TestQgsTaskManager::addTask()
368371
QCOMPARE( task2->status(), QgsTask::Running );
369372

370373
QCOMPARE( spy.count(), 2 );
371-
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
374+
QCOMPARE( spy.last().at( 0 ).toLongLong(), 2LL );
372375

373376
task->cancel();
374377
task2->cancel();
@@ -649,8 +652,8 @@ void TestQgsTaskManager::taskId()
649652
TestTask *task3 = new TestTask();
650653

651654
QCOMPARE( manager.taskId( nullptr ), -1L );
652-
QCOMPARE( manager.taskId( task ), 0L );
653-
QCOMPARE( manager.taskId( task2 ), 1L );
655+
QCOMPARE( manager.taskId( task ), 1L );
656+
QCOMPARE( manager.taskId( task2 ), 2L );
654657
QCOMPARE( manager.taskId( task3 ), -1L );
655658

656659
delete task3;
@@ -725,15 +728,15 @@ void TestQgsTaskManager::progressChanged()
725728

726729
QCOMPARE( task->progress(), 50.0 );
727730
QCOMPARE( spy.count(), 1 );
728-
QCOMPARE( spy.last().at( 0 ).toLongLong(), 0LL );
731+
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
729732
QCOMPARE( spy.last().at( 1 ).toDouble(), 50.0 );
730733
//multiple running tasks, so finalTaskProgressChanged(double) should not be emitted
731734
QCOMPARE( spy2.count(), 0 );
732735

733736
task2->emitProgressChanged( 75.0 );
734737
QCOMPARE( task2->progress(), 75.0 );
735738
QCOMPARE( spy.count(), 2 );
736-
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
739+
QCOMPARE( spy.last().at( 0 ).toLongLong(), 2LL );
737740
QCOMPARE( spy.last().at( 1 ).toDouble(), 75.0 );
738741
QCOMPARE( spy2.count(), 0 );
739742

@@ -799,7 +802,7 @@ void TestQgsTaskManager::statusChanged()
799802
flushEvents();
800803

801804
QCOMPARE( spy.count(), 1 );
802-
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
805+
QCOMPARE( spy.last().at( 0 ).toLongLong(), 2LL );
803806
QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Running );
804807

805808
task->terminate();
@@ -809,7 +812,7 @@ void TestQgsTaskManager::statusChanged()
809812
}
810813
flushEvents();
811814
QCOMPARE( spy.count(), 2 );
812-
QCOMPARE( spy.last().at( 0 ).toLongLong(), 0LL );
815+
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
813816
QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Terminated );
814817

815818
task2->finish();
@@ -819,7 +822,7 @@ void TestQgsTaskManager::statusChanged()
819822
}
820823
flushEvents();
821824
QCOMPARE( spy.count(), 3 );
822-
QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
825+
QCOMPARE( spy.last().at( 0 ).toLongLong(), 2LL );
823826
QCOMPARE( static_cast< QgsTask::TaskStatus >( spy.last().at( 1 ).toInt() ), QgsTask::Complete );
824827
}
825828

@@ -1126,7 +1129,7 @@ void TestQgsTaskManager::managerWithSubTasks()
11261129
QCOMPARE( spyProgress.count(), 0 );
11271130
subTask->emitProgressChanged( 50 );
11281131
QCOMPARE( spyProgress.count(), 1 );
1129-
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 0LL );
1132+
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 1LL );
11301133
// subTask itself is 50% done, so with it's child task it's sitting at overall 25% done
11311134
// (one task 50%, one task not started)
11321135
// parent task has two tasks (itself + subTask), and subTask is 25% done.... so parent
@@ -1135,11 +1138,11 @@ void TestQgsTaskManager::managerWithSubTasks()
11351138

11361139
subsubTask->emitProgressChanged( 100 );
11371140
QCOMPARE( spyProgress.count(), 2 );
1138-
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 0LL );
1141+
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 1LL );
11391142
QCOMPARE( spyProgress.last().at( 1 ).toInt(), 38 );
11401143
parent->emitProgressChanged( 50 );
11411144
QCOMPARE( spyProgress.count(), 3 );
1142-
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 0LL );
1145+
QCOMPARE( spyProgress.last().at( 0 ).toLongLong(), 1LL );
11431146
QCOMPARE( spyProgress.last().at( 1 ).toInt(), 63 );
11441147

11451148
//manager should not emit statusChanged signals for subtasks
@@ -1152,7 +1155,7 @@ void TestQgsTaskManager::managerWithSubTasks()
11521155
}
11531156
flushEvents();
11541157
QCOMPARE( statusSpy.count(), 1 );
1155-
QCOMPARE( statusSpy.last().at( 0 ).toLongLong(), 0LL );
1158+
QCOMPARE( statusSpy.last().at( 0 ).toLongLong(), 1LL );
11561159
QCOMPARE( static_cast< QgsTask::TaskStatus >( statusSpy.last().at( 1 ).toInt() ), QgsTask::Running );
11571160

11581161
subTask->finish();
@@ -1170,7 +1173,7 @@ void TestQgsTaskManager::managerWithSubTasks()
11701173
}
11711174
flushEvents();
11721175
QCOMPARE( statusSpy.count(), 2 );
1173-
QCOMPARE( statusSpy.last().at( 0 ).toLongLong(), 0LL );
1176+
QCOMPARE( statusSpy.last().at( 0 ).toLongLong(), 1LL );
11741177
QCOMPARE( static_cast< QgsTask::TaskStatus >( statusSpy.last().at( 1 ).toInt() ), QgsTask::Complete );
11751178

11761179

0 commit comments

Comments
 (0)
Please sign in to comment.