Fix thread safety in raster layer rendering

We were cloning the data provider correctly, but doing this on the main thread means that the provider has thread affinity with the main thread -- so we need to ensure it is moved over to the actual rendering thread prior to the rendering.
qgis · Dec 15, 2022 · 922dd94 · 922dd94
1 parent efb17e8
commit 922dd94
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 0 deletions.
diff --git a/python/core/auto_generated/raster/qgsrasterpipe.sip.in b/python/core/auto_generated/raster/qgsrasterpipe.sip.in
@@ -37,6 +37,21 @@ Constructor for an empty QgsRasterPipe.
     ~QgsRasterPipe();
 
 
+    void moveToThread( QThread *thread );
+%Docstring
+Moves the pipe to another ``thread``.
+
+This effects all QObject derived interfaces in the pipe, and follows the same
+behavior as QObject.moveToThread. Specifically, it is permitted to PUSH the
+pipe from the current thread to another thread, but NOT to PULL a pipe
+from another thread over to the current thread. Pulling is only supported
+by first pushsing the pipe from its current thread to a ``None`` thread,
+and then later pulling to the current thread. See QObject documentation
+for more details.
+
+.. versionadded:: 3.30
+%End
+
     bool insert( int idx, QgsRasterInterface *interface /Transfer/ );
 %Docstring
 Attempts to insert interface at specified index and connect

diff --git a/src/core/raster/qgsrasterlayerrenderer.cpp b/src/core/raster/qgsrasterlayerrenderer.cpp
@@ -30,6 +30,7 @@
 
 #include <QElapsedTimer>
 #include <QPointer>
+#include <QThread>
 
 ///@cond PRIVATE
 
@@ -284,6 +285,8 @@ QgsRasterLayerRenderer::QgsRasterLayerRenderer( QgsRasterLayer *layer, QgsRender
   mClippingRegions = QgsMapClippingUtils::collectClippingRegionsForLayer( *renderContext(), layer );
 
   mFeedback->setRenderContext( rendererContext );
+
+  mPipe->moveToThread( nullptr );
 }
 
 QgsRasterLayerRenderer::~QgsRasterLayerRenderer()
@@ -302,6 +305,8 @@ bool QgsRasterLayerRenderer::render()
                                 QgsRasterInterface::Capability::Prefetch ) ) )
     return true;
 
+  mPipe->moveToThread( QThread::currentThread() );
+
   QElapsedTimer time;
   time.start();
   //
@@ -361,6 +366,8 @@ bool QgsRasterLayerRenderer::render()
   QgsDebugMsgLevel( QStringLiteral( "total raster draw time (ms):     %1" ).arg( time.elapsed(), 5 ), 4 );
   mReadyToCompose = true;
 
+  mPipe->moveToThread( nullptr );
+
   return !mFeedback->isCanceled();
 }
 

diff --git a/src/core/raster/qgsrasterpipe.cpp b/src/core/raster/qgsrasterpipe.cpp
@@ -63,6 +63,19 @@ QgsRasterPipe::~QgsRasterPipe()
   }
 }
 
+void QgsRasterPipe::moveToThread( QThread *thread )
+{
+  // only data provider is derived from QObject currently:
+  auto it = mRoleMap.find( Qgis::RasterPipeInterfaceRole::Provider );
+  if ( it != mRoleMap.end() )
+  {
+    if ( QgsRasterDataProvider *dp = dynamic_cast<QgsRasterDataProvider *>( mInterfaces.value( it.value() ) ) )
+    {
+      dp->moveToThread( thread );
+    }
+  }
+}
+
 bool QgsRasterPipe::connect( QVector<QgsRasterInterface *> interfaces )
 {
   QgsDebugMsgLevel( QStringLiteral( "Entered" ), 4 );

diff --git a/src/core/raster/qgsrasterpipe.h b/src/core/raster/qgsrasterpipe.h
@@ -73,6 +73,21 @@ class CORE_EXPORT QgsRasterPipe
 
     QgsRasterPipe &operator=( const QgsRasterPipe &rh ) = delete;
 
+    /**
+     * Moves the pipe to another \a thread.
+     *
+     * This effects all QObject derived interfaces in the pipe, and follows the same
+     * behavior as QObject::moveToThread. Specifically, it is permitted to PUSH the
+     * pipe from the current thread to another thread, but NOT to PULL a pipe
+     * from another thread over to the current thread. Pulling is only supported
+     * by first pushsing the pipe from its current thread to a NULLPTR thread,
+     * and then later pulling to the current thread. See QObject documentation
+     * for more details.
+     *
+     * \since QGIS 3.30
+    */
+    void moveToThread( QThread *thread );
+
     /**
      * Attempts to insert interface at specified index and connect
      * if connection would fail, the interface is not inserted and FALSE is returned