Skip to content

Commit

Permalink
Safer GDAL delete and close of datasets, which doesn't leak
Browse files Browse the repository at this point in the history
and works on Windows

See
d024910#commitcomment-25194282
  • Loading branch information
nyalldawson committed Oct 25, 2017
1 parent 8eb919e commit 332215c
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 5 deletions.
3 changes: 1 addition & 2 deletions src/analysis/raster/qgsninecellfilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/raster/qgsrastercalculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down
3 changes: 1 addition & 2 deletions src/analysis/raster/qgsrelief.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
21 changes: 21 additions & 0 deletions src/core/qgsogrutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 );
Expand Down
10 changes: 10 additions & 0 deletions src/core/qgsogrutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down

0 comments on commit 332215c

Please sign in to comment.