Safer GDAL delete and close of datasets, which doesn't leak

and works on Windows See d024910#commitcomment-25194282
qgis · Oct 25, 2017 · 332215c · 332215c
1 parent 8eb919e
commit 332215c
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 5 deletions.
diff --git a/src/analysis/raster/qgsninecellfilter.cpp b/src/analysis/raster/qgsninecellfilter.cpp
@@ -166,8 +166,7 @@ int QgsNineCellFilter::processRaster( QgsFeedback *feedback )
   if ( feedback && feedback->isCanceled() )
   {
     //delete the dataset without closing (because it is faster)
-    ( void )outputDataset.release();
-    GDALDeleteDataset( outputDriver, mOutputFile.toUtf8().constData() );
+    gdal::fast_delete_and_close( outputDataset, outputDriver, mOutputFile );
     return 7;
   }
   return 0;

diff --git a/src/analysis/raster/qgsrastercalculator.cpp b/src/analysis/raster/qgsrastercalculator.cpp
@@ -168,7 +168,7 @@ int QgsRasterCalculator::processCalculation( QgsFeedback *feedback )
   if ( feedback && feedback->isCanceled() )
   {
     //delete the dataset without closing (because it is faster)
-    GDALDeleteDataset( outputDriver, mOutputFile.toUtf8().constData() );
+    gdal::fast_delete_and_close( outputDataset, outputDriver, mOutputFile );
     return static_cast< int >( Canceled );
   }
   return static_cast< int >( Success );

diff --git a/src/analysis/raster/qgsrelief.cpp b/src/analysis/raster/qgsrelief.cpp
@@ -276,8 +276,7 @@ int QgsRelief::processRaster( QgsFeedback *feedback )
   if ( feedback && feedback->isCanceled() )
   {
     //delete the dataset without closing (because it is faster)
-    ( void )outputDataset.release();
-    GDALDeleteDataset( outputDriver, mOutputFile.toUtf8().constData() );
+    gdal::fast_delete_and_close( outputDataset, outputDriver, mOutputFile );
     return 7;
   }
 

diff --git a/src/core/qgsogrutils.cpp b/src/core/qgsogrutils.cpp
@@ -20,6 +20,7 @@
 #include "qgsfields.h"
 #include <QTextCodec>
 #include <QUuid>
+#include <cpl_error.h>
 
 // Starting with GDAL 2.2, there are 2 concepts: unset fields and null fields
 // whereas previously there was only unset fields. For QGIS purposes, both
@@ -56,6 +57,26 @@ void gdal::GDALDatasetCloser::operator()( void *dataset )
   GDALClose( dataset );
 }
 
+void gdal::fast_delete_and_close( gdal::dataset_unique_ptr &dataset, GDALDriverH driver, const QString &path )
+{
+  // see https://github.com/qgis/QGIS/commit/d024910490a39e65e671f2055c5b6543e06c7042#commitcomment-25194282
+  // faster if we close the handle AFTER delete, but doesn't work for windows
+#ifdef Q_OS_WIN
+  // close dataset handle
+  dataset.reset();
+#endif
+
+  CPLPushErrorHandler( CPLQuietErrorHandler );
+  GDALDeleteDataset( driver, path.toUtf8().constData() );
+  CPLPopErrorHandler();
+
+#ifndef Q_OS_WIN
+  // close dataset handle
+  dataset.reset();
+#endif
+}
+
+
 void gdal::GDALWarpOptionsDeleter::operator()( GDALWarpOptions *options )
 {
   GDALDestroyWarpOptions( options );

diff --git a/src/core/qgsogrutils.h b/src/core/qgsogrutils.h
@@ -133,6 +133,16 @@ namespace gdal
    */
   using dataset_unique_ptr = std::unique_ptr< void, GDALDatasetCloser >;
 
+  /**
+   * Performs a fast close of an unwanted GDAL dataset handle by deleting the underlying
+   * data store. Use when the resultant dataset is no longer required, e.g. as a result
+   * of user cancelation of an operation.
+   *
+   * Requires a gdal \a dataset pointer, the corresponding gdal \driver and underlying
+   * dataset file \a path.
+   */
+  void CORE_EXPORT fast_delete_and_close( dataset_unique_ptr &dataset, GDALDriverH driver, const QString &path );
+
   /**
    * Scoped GDAL warp options.
    */