Fix crash when tasks are canceled before starting

Avoids an attempt to unlock an already unlocked mutex, which causes a crash. Should fix a number of crash report tickets...
qgis · Aug 30, 2018 · d683550 · d683550
1 parent 123f694
commit d683550
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 1 deletion.
diff --git a/src/core/qgstaskmanager.cpp b/src/core/qgstaskmanager.cpp
@@ -34,11 +34,12 @@ QgsTask::QgsTask( const QString &name, Flags flags )
 QgsTask::~QgsTask()
 {
   Q_ASSERT_X( mStatus != Running, "delete", QStringLiteral( "status was %1" ).arg( mStatus ).toLatin1() );
-
+  mNotFinishedMutex.tryLock(); // we're not guaranteed to already have the lock in place here
   Q_FOREACH ( const SubTask &subTask, mSubTasks )
   {
     delete subTask.task;
   }
+  mNotFinishedMutex.unlock();
 }
 
 qint64 QgsTask::elapsedTime() const
@@ -261,6 +262,7 @@ void QgsTask::processSubTasksForCompletion()
     setProgress( 100.0 );
     emit statusChanged( Complete );
     emit taskCompleted();
+    mNotFinishedMutex.tryLock(); // we're not guaranteed to already have the lock in place here
     mNotFinishedMutex.unlock();
   }
   else if ( mStatus == Complete )
@@ -288,6 +290,7 @@ void QgsTask::processSubTasksForTermination()
 
     emit statusChanged( Terminated );
     emit taskTerminated();
+    mNotFinishedMutex.tryLock(); // we're not guaranteed to already have the lock in place here
     mNotFinishedMutex.unlock();
   }
   else if ( mStatus == Terminated && !subTasksTerminated )

diff --git a/tests/src/core/testqgstaskmanager.cpp b/tests/src/core/testqgstaskmanager.cpp
@@ -269,6 +269,7 @@ class TestQgsTaskManager : public QObject
     void managerWithSubTasks();
     void managerWithSubTasks2();
     void managerWithSubTasks3();
+    void cancelBeforeStart();
 };
 
 void TestQgsTaskManager::initTestCase()
@@ -1348,5 +1349,30 @@ void TestQgsTaskManager::managerWithSubTasks3()
   QCOMPARE( manager3.dependencies( subTask2Id ), QSet< long >() );
 }
 
+void TestQgsTaskManager::cancelBeforeStart()
+{
+  // add a lot of tasks to the manager, so that some are queued and can't start immediately
+  // then cancel them all!
+  QList< QgsTask * > tasks;
+  QgsTaskManager manager;
+  for ( int i = 0; i < 30; ++i )
+  {
+    QgsTask *task = new CancelableTask();
+    tasks << task;
+    manager.addTask( task );
+  }
+
+  for ( QgsTask *t : qgis::as_const( tasks ) )
+  {
+    t->cancel();
+  }
+
+  while ( manager.countActiveTasks() > 1 )
+  {
+    QCoreApplication::processEvents();
+  }
+  flushEvents();
+}
+
 QGSTEST_MAIN( TestQgsTaskManager )
 #include "testqgstaskmanager.moc"