Merge pull request #5230 from m-kuhn/taskWaitForFinished

QgsTask::waitForFinished without event loop

python/core/qgstaskmanager.sip

@@ -170,10 +170,10 @@ class QgsTask : QObject
  :rtype: list of QgsMapLayer
 %End
 
-    bool waitForFinished( int timeout = 30000 );
+    bool waitForFinished( unsigned long timeout = 30000 );
 %Docstring
  Blocks the current thread until the task finishes or a maximum of ``timeout`` milliseconds.
- If the ``timeout`` is ``-1`` the thread will be blocked forever.
+ If ``timeout`` is ``0`` the thread will be blocked forever.
  In case of a timeout, the task will still be running.
  In case the task already is finished, the method will return immediately while
  returning ``true``.

src/core/qgstaskmanager.cpp

@@ -35,7 +35,9 @@ QgsTask::QgsTask( const QString &name, const Flags &flags )
   , mTotalProgress( 0.0 )
   , mShouldTerminate( false )
   , mStartCount( 0 )
-{}
+{
+  mNotFinishedMutex.lock();
+}
 
 QgsTask::~QgsTask()
 {
@@ -142,26 +144,19 @@ QList<QgsMapLayer *> QgsTask::dependentLayers() const
   return _qgis_listQPointerToRaw( mDependentLayers );
 }
 
-bool QgsTask::waitForFinished( int timeout )
+bool QgsTask::waitForFinished( unsigned long timeout )
 {
-  QEventLoop loop;
   bool rv = true;
-
-  connect( this, &QgsTask::taskCompleted, &loop, &QEventLoop::quit );
-  connect( this, &QgsTask::taskTerminated, &loop, &QEventLoop::quit );
-  QTimer timer;
-
-  if ( timeout != -1 )
+  if ( mOverallStatus == Complete || mOverallStatus == Terminated )
   {
-    timer.start( timeout );
-    connect( &timer, &QTimer::timeout, [&rv]() { rv = false; } );
-    connect( &timer, &QTimer::timeout, &loop, &QEventLoop::quit );
+    rv = true;
+  }
+  else
+  {
+    if ( timeout == 0 )
+      timeout = ULONG_MAX;
+    rv = mTaskFinished.wait( &mNotFinishedMutex, timeout );
   }
-
-  if ( status() == QgsTask::Complete || status() == QgsTask::Terminated )
-    return true;
-  loop.exec();
-
   return rv;
 }
 
@@ -249,9 +244,13 @@ void QgsTask::processSubTasksForCompletion()
   if ( mStatus == Complete && subTasksCompleted )
   {
     mOverallStatus = Complete;
+
     setProgress( 100.0 );
     emit statusChanged( Complete );
     emit taskCompleted();
+    mTaskFinished.wakeAll();
+    mNotFinishedMutex.unlock();
+    mTaskFinished.wakeAll();
   }
   else if ( mStatus == Complete )
   {
@@ -275,8 +274,12 @@ void QgsTask::processSubTasksForTermination()
   if ( mStatus == Terminated && subTasksTerminated && mOverallStatus != Terminated )
   {
     mOverallStatus = Terminated;
+
     emit statusChanged( Terminated );
     emit taskTerminated();
+    mTaskFinished.wakeAll();
+    mNotFinishedMutex.unlock();
+    mTaskFinished.wakeAll();
   }
   else if ( mStatus == Terminated && !subTasksTerminated )
   {

src/core/qgstaskmanager.h

@@ -188,14 +188,14 @@ class CORE_EXPORT QgsTask : public QObject
 
     /**
      * Blocks the current thread until the task finishes or a maximum of \a timeout milliseconds.
-     * If the \a timeout is ``-1`` the thread will be blocked forever.
+     * If \a timeout is ``0`` the thread will be blocked forever.
      * In case of a timeout, the task will still be running.
      * In case the task already is finished, the method will return immediately while
      * returning ``true``.
      *
      * The result will be false if the wait timed out and true in any other case.
      */
-    bool waitForFinished( int timeout = 30000 );
+    bool waitForFinished( unsigned long timeout = 30000 );
 
   signals:
 
@@ -290,6 +290,10 @@ class CORE_EXPORT QgsTask : public QObject
     //! Status of this task and all subtasks
     TaskStatus mOverallStatus;
 
+    //! This mutex remains locked from initialization until the task finishes,
+    //! it's used as a trigger for waitForFinished.
+    QMutex mNotFinishedMutex;
+
     //! Progress of this (parent) task alone
     double mProgress;
     //! Overall progress of this task and all subtasks
@@ -297,6 +301,8 @@ class CORE_EXPORT QgsTask : public QObject
     bool mShouldTerminate;
     int mStartCount;
 
+    QWaitCondition mTaskFinished;
+
     struct SubTask
     {
       SubTask( QgsTask *task, QgsTaskList dependencies, SubTaskDependency dependency )

tests/src/python/test_qgsactionmanager.py

@@ -154,8 +154,7 @@ class TestQgsActionManager(unittest.TestCase):
         return output
 
     @unittest.expectedFailure(platform.system() != 'Linux')
-    @unittest.skipIf(
-        os.environ.get('TRAVIS', '') == 'true', 'Test is flaky on Travis environment')
+    @unittest.skipIf(os.environ.get('TRAVIS', '') == 'true', 'Test is flaky on Travis environment')
     def testDoAction(self):
         """ test running action """
 

tests/src/python/test_qgsserver_wms.py

@@ -64,6 +64,7 @@ class TestQgsServerWMS(QgsServerTestBase):
 
         self.assertXMLEqual(response, expected, msg="request %s failed.\nQuery: %s\nExpected file: %s\nResponse:\n%s" % (query_string, request, reference_path, response.decode('utf-8')))
 
+    @unittest.skipIf(os.environ.get('TRAVIS', '') == 'true', 'Test is flaky on Travis environment')
     def test_project_wms(self):
         """Test some WMS request"""
         for request in ('GetCapabilities', 'GetProjectSettings', 'GetContext'):