Fix leak of label engine when start called on single render job multiple times

Author: nyalldawson

src/core/qgslabelingengine.cpp

@@ -77,14 +77,11 @@ QgsLabelingEngine::QgsLabelingEngine()
     , mCandPoint( 16 )
     , mCandLine( 50 )
     , mCandPolygon( 30 )
-    , mResults( nullptr )
-{
-  mResults = new QgsLabelingResults;
-}
+    , mResults( new QgsLabelingResults )
+{}
 
 QgsLabelingEngine::~QgsLabelingEngine()
 {
-  delete mResults;
   qDeleteAll( mProviders );
   qDeleteAll( mSubProviders );
 }
@@ -358,9 +355,7 @@ void QgsLabelingEngine::run( QgsRenderContext& context )
 
 QgsLabelingResults* QgsLabelingEngine::takeResults()
 {
-  QgsLabelingResults* res = mResults;
-  mResults = nullptr;
-  return res;
+  return mResults.release();
 }
 
 

src/core/qgslabelingengine.h

@@ -210,7 +210,7 @@ class CORE_EXPORT QgsLabelingEngine
     QgsLabelingResults* takeResults();
 
     //! For internal use by the providers
-    QgsLabelingResults* results() const { return mResults; }
+    QgsLabelingResults* results() const { return mResults.get(); }
 
     //! Set flags of the labeling engine
     void setFlags( Flags flags ) { mFlags = flags; }
@@ -255,7 +255,7 @@ class CORE_EXPORT QgsLabelingEngine
     int mCandPoint, mCandLine, mCandPolygon;
 
     //! Resulting labeling layout
-    QgsLabelingResults* mResults;
+    std::unique_ptr< QgsLabelingResults > mResults;
 
 };
 

src/core/qgsmaprenderercustompainterjob.cpp

@@ -28,7 +28,6 @@
 QgsMapRendererCustomPainterJob::QgsMapRendererCustomPainterJob( const QgsMapSettings& settings, QPainter* painter )
     : QgsMapRendererJob( settings )
     , mPainter( painter )
-    , mLabelingEngineV2( nullptr )
     , mActive( false )
     , mRenderSynchronously( false )
 {
@@ -40,9 +39,6 @@ QgsMapRendererCustomPainterJob::~QgsMapRendererCustomPainterJob()
   QgsDebugMsg( "QPAINTER destruct" );
   Q_ASSERT( !mFutureWatcher.isRunning() );
   //cancel();
-
-  delete mLabelingEngineV2;
-  mLabelingEngineV2 = nullptr;
 }
 
 void QgsMapRendererCustomPainterJob::start()
@@ -73,19 +69,18 @@ void QgsMapRendererCustomPainterJob::start()
   Q_ASSERT_X( qgsDoubleNear( thePaintDevice->logicalDpiX(), mSettings.outputDpi() ), "Job::startRender()", errMsg.toLatin1().data() );
 #endif
 
-  delete mLabelingEngineV2;
-  mLabelingEngineV2 = nullptr;
+  mLabelingEngineV2.reset();
 
   if ( mSettings.testFlag( QgsMapSettings::DrawLabeling ) )
   {
-    mLabelingEngineV2 = new QgsLabelingEngine();
+    mLabelingEngineV2.reset( new QgsLabelingEngine() );
     mLabelingEngineV2->readSettingsFromProject( QgsProject::instance() );
     mLabelingEngineV2->setMapSettings( mSettings );
   }
 
   bool canUseLabelCache = prepareLabelCache();
-  mLayerJobs = prepareJobs( mPainter, mLabelingEngineV2 );
-  mLabelJob = prepareLabelingJob( mPainter, mLabelingEngineV2, canUseLabelCache );
+  mLayerJobs = prepareJobs( mPainter, mLabelingEngineV2.get() );
+  mLabelJob = prepareLabelingJob( mPainter, mLabelingEngineV2.get(), canUseLabelCache );
 
   QgsDebugMsg( "Rendering prepared in (seconds): " + QString( "%1" ).arg( prepareTime.elapsed() / 1000.0 ) );
 
@@ -284,12 +279,12 @@ void QgsMapRendererCustomPainterJob::doRender()
         mLabelJob.img->fill( 0 );
         painter.begin( mLabelJob.img );
         mLabelJob.context.setPainter( &painter );
-        drawLabeling( mSettings, mLabelJob.context, mLabelingEngineV2, &painter );
+        drawLabeling( mSettings, mLabelJob.context, mLabelingEngineV2.get(), &painter );
         painter.end();
       }
       else
       {
-        drawLabeling( mSettings, mLabelJob.context, mLabelingEngineV2, mPainter );
+        drawLabeling( mSettings, mLabelJob.context, mLabelingEngineV2.get(), mPainter );
       }
 
       mLabelJob.complete = true;

src/core/qgsmaprenderercustompainterjob.h

@@ -84,7 +84,7 @@ class CORE_EXPORT QgsMapRendererCustomPainterJob : public QgsMapRendererJob
     QPainter* mPainter;
     QFuture<void> mFuture;
     QFutureWatcher<void> mFutureWatcher;
-    QgsLabelingEngine* mLabelingEngineV2;
+    std::unique_ptr< QgsLabelingEngine > mLabelingEngineV2;
 
     bool mActive;
     LayerRenderJobs mLayerJobs;

src/core/qgsmaprendererparalleljob.cpp

@@ -29,7 +29,6 @@
 QgsMapRendererParallelJob::QgsMapRendererParallelJob( const QgsMapSettings& settings )
     : QgsMapRendererQImageJob( settings )
     , mStatus( Idle )
-    , mLabelingEngineV2( nullptr )
 {
 }
 
@@ -39,9 +38,6 @@ QgsMapRendererParallelJob::~QgsMapRendererParallelJob()
   {
     cancel();
   }
-
-  delete mLabelingEngineV2;
-  mLabelingEngineV2 = nullptr;
 }
 
 void QgsMapRendererParallelJob::start()
@@ -53,19 +49,18 @@ void QgsMapRendererParallelJob::start()
 
   mStatus = RenderingLayers;
 
-  delete mLabelingEngineV2;
-  mLabelingEngineV2 = nullptr;
+  mLabelingEngineV2.reset();
 
   if ( mSettings.testFlag( QgsMapSettings::DrawLabeling ) )
   {
-    mLabelingEngineV2 = new QgsLabelingEngine();
+    mLabelingEngineV2.reset( new QgsLabelingEngine() );
     mLabelingEngineV2->readSettingsFromProject( QgsProject::instance() );
     mLabelingEngineV2->setMapSettings( mSettings );
   }
 
   bool canUseLabelCache = prepareLabelCache();
-  mLayerJobs = prepareJobs( nullptr, mLabelingEngineV2 );
-  mLabelJob = prepareLabelingJob( nullptr, mLabelingEngineV2, canUseLabelCache );
+  mLayerJobs = prepareJobs( nullptr, mLabelingEngineV2.get() );
+  mLabelJob = prepareLabelingJob( nullptr, mLabelingEngineV2.get(), canUseLabelCache );
 
   QgsDebugMsg( QString( "QThreadPool max thread count is %1" ).arg( QThreadPool::globalInstance()->maxThreadCount() ) );
 
@@ -280,7 +275,7 @@ void QgsMapRendererParallelJob::renderLabelsStatic( QgsMapRendererParallelJob* s
     // draw the labels!
     try
     {
-      drawLabeling( self->mSettings, job.context, self->mLabelingEngineV2, &painter );
+      drawLabeling( self->mSettings, job.context, self->mLabelingEngineV2.get(), &painter );
     }
     catch ( QgsException & e )
     {

src/core/qgsmaprendererparalleljob.h

@@ -70,7 +70,7 @@ class CORE_EXPORT QgsMapRendererParallelJob : public QgsMapRendererQImageJob
     LabelRenderJob mLabelJob;
 
     //! New labeling engine
-    QgsLabelingEngine* mLabelingEngineV2;
+    std::unique_ptr< QgsLabelingEngine > mLabelingEngineV2;
     QFuture<void> mLabelingFuture;
     QFutureWatcher<void> mLabelingFutureWatcher;
 

src/core/qgsmaprenderersequentialjob.cpp

@@ -23,7 +23,6 @@ QgsMapRendererSequentialJob::QgsMapRendererSequentialJob( const QgsMapSettings&
     : QgsMapRendererQImageJob( settings )
     , mInternalJob( nullptr )
     , mPainter( nullptr )
-    , mLabelingResults( nullptr )
 {
   QgsDebugMsg( "SEQUENTIAL construct" );
 
@@ -44,9 +43,6 @@ QgsMapRendererSequentialJob::~QgsMapRendererSequentialJob()
   }
 
   Q_ASSERT( !mInternalJob && !mPainter );
-
-  delete mLabelingResults;
-  mLabelingResults = nullptr;
 }
 
 
@@ -55,6 +51,8 @@ void QgsMapRendererSequentialJob::start()
   if ( isActive() )
     return; // do nothing if we are already running
 
+  mLabelingResults.reset();
+
   mRenderingStart.start();
 
   mErrors.clear();
@@ -105,9 +103,7 @@ bool QgsMapRendererSequentialJob::usedCachedLabels() const
 
 QgsLabelingResults* QgsMapRendererSequentialJob::takeLabelingResults()
 {
-  QgsLabelingResults* tmp = mLabelingResults;
-  mLabelingResults = nullptr;
-  return tmp;
+  return mLabelingResults.release();
 }
 
 
@@ -129,7 +125,7 @@ void QgsMapRendererSequentialJob::internalFinished()
   delete mPainter;
   mPainter = nullptr;
 
-  mLabelingResults = mInternalJob->takeLabelingResults();
+  mLabelingResults.reset( mInternalJob->takeLabelingResults() );
   mUsedCachedLabels = mInternalJob->usedCachedLabels();
 
   mErrors = mInternalJob->errors();

src/core/qgsmaprenderersequentialjob.h

@@ -56,7 +56,7 @@ class CORE_EXPORT QgsMapRendererSequentialJob : public QgsMapRendererQImageJob
     QgsMapRendererCustomPainterJob* mInternalJob;
     QImage mImage;
     QPainter* mPainter;
-    QgsLabelingResults* mLabelingResults;
+    std::unique_ptr< QgsLabelingResults > mLabelingResults;
     bool mUsedCachedLabels = false;
 
 };