Fix crash when loading WCS layers (fixes #15595)

wonder-sk · wonder-sk · commit 08f4a0f40cce · 2016-10-05T09:19:43.000+08:00
The problem is that some providers would still issue network
requests in prepareJobs() - this should be ideally avoided,
because it is run in main thread - all the work should be deferred
to be done in worker thread.
diff --git a/src/core/qgsmaprenderercustompainterjob.cpp b/src/core/qgsmaprenderercustompainterjob.cpp
@@ -98,14 +98,6 @@ void QgsMapRendererCustomPainterJob::start()
   }
 
   mLayerJobs = prepareJobs( mPainter, mLabelingEngine, mLabelingEngineV2 );
-  // prepareJobs calls mapLayer->createMapRenderer may involve cloning a RasterDataProvider,
-  // whose constructor may need to download some data (i.e. WMS, AMS) and doing so runs a
-  // QEventLoop waiting for the network request to complete. If unluckily someone calls
-  // mapCanvas->refresh() while this is happening, QgsMapRendererCustomPainterJob::cancel is
-  // called, deleting the QgsMapRendererCustomPainterJob while this function is running.
-  // Hence we need to check whether the job is still active before proceeding
-  if ( !isActive() )
-    return;
 
   QgsDebugMsg( "Rendering prepared in (seconds): " + QString( "%1" ).arg( prepareTime.elapsed() / 1000.0 ) );
 
diff --git a/src/core/qgsmaprendererparalleljob.cpp b/src/core/qgsmaprendererparalleljob.cpp
@@ -76,14 +76,6 @@ void QgsMapRendererParallelJob::start()
   }
 
   mLayerJobs = prepareJobs( nullptr, mLabelingEngine, mLabelingEngineV2 );
-  // prepareJobs calls mapLayer->createMapRenderer may involve cloning a RasterDataProvider,
-  // whose constructor may need to download some data (i.e. WMS, AMS) and doing so runs a
-  // QEventLoop waiting for the network request to complete. If unluckily someone calls
-  // mapCanvas->refresh() while this is happening, QgsMapRendererCustomPainterJob::cancel is
-  // called, deleting the QgsMapRendererCustomPainterJob while this function is running.
-  // Hence we need to check whether the job is still active before proceeding
-  if ( !isActive() )
-    return;
 
   QgsDebugMsg( QString( "QThreadPool max thread count is %1" ).arg( QThreadPool::globalInstance()->maxThreadCount() ) );
 
diff --git a/src/gui/qgsmapcanvas.cpp b/src/gui/qgsmapcanvas.cpp
@@ -706,9 +706,6 @@ void QgsMapCanvas::refreshMap()
 
   stopRendering(); // if any...
 
-  // from now on we can accept refresh requests again
-  mRefreshScheduled = false;
-
   //build the expression context
   QgsExpressionContext expressionContext;
   expressionContext << QgsExpressionContextUtils::globalScope()
@@ -741,6 +738,14 @@ void QgsMapCanvas::refreshMap()
 
   mJob->start();
 
+  // from now on we can accept refresh requests again
+  // this must be reset only after the job has been started, because
+  // some providers (yes, it's you WCS and AMS!) during preparation
+  // do network requests and start an internal event loop, which may
+  // end up calling refresh() and would schedule another refresh,
+  // deleting the one we have just started.
+  mRefreshScheduled = false;
+
   mMapUpdateTimer.start();
 
   emit renderStarting();
