Skip to content

Commit

Permalink
[GDAL provider] Revise how referencing counting is done on mGdalBaseD…
Browse files Browse the repository at this point in the history
…ataset

initBaseDataset() used to take a reference in the case where
mGdalDataset == mGdalBaseDataset (non warped VRT) and we dropped it when
closing the dataset, which was OK

However buidPyramids() failed to acquire this reference. There was no
negative consequence as GDALDerefenceDataset() just decremented a
reference count, and GDALClose() ignores it for a non-shared dataset,
however this was an incorrected use

It is simpler for the mind to call GDALDerefenceDataset() only when
mGdalBaseDataset != mGdalDataset.
  • Loading branch information
rouault committed Oct 14, 2018
1 parent e7d15b6 commit 651ccb4
Showing 1 changed file with 8 additions and 12 deletions.
20 changes: 8 additions & 12 deletions src/providers/gdal/qgsgdalprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ bool QgsGdalProvider::cacheGdalHandlesForLaterReuse( QgsGdalProvider *provider,
{
mgDatasetCacheSize --;
DatasetPair pair = mgDatasetCache[ candidateProvider ].takeLast();
if ( pair.mGdalBaseDataset )
if ( pair.mGdalBaseDataset != pair.mGdalDataset )
{
GDALDereferenceDataset( pair.mGdalBaseDataset );
}
Expand Down Expand Up @@ -435,7 +435,7 @@ void QgsGdalProvider::closeCachedGdalHandlesFor( QgsGdalProvider *provider )
{
mgDatasetCacheSize --;
DatasetPair pair = iter.value().takeLast();
if ( pair.mGdalBaseDataset )
if ( pair.mGdalBaseDataset != pair.mGdalDataset )
{
GDALDereferenceDataset( pair.mGdalBaseDataset );
}
Expand Down Expand Up @@ -464,7 +464,7 @@ QgsGdalProvider::~QgsGdalProvider()
}
else
{
if ( mGdalBaseDataset )
if ( mGdalBaseDataset != mGdalDataset )
{
GDALDereferenceDataset( mGdalBaseDataset );
}
Expand Down Expand Up @@ -498,7 +498,10 @@ void QgsGdalProvider::closeDataset()
}
mValid = false;

GDALDereferenceDataset( mGdalBaseDataset );
if ( mGdalBaseDataset != mGdalDataset )
{
GDALDereferenceDataset( mGdalBaseDataset );
}
mGdalBaseDataset = nullptr;

GDALClose( mGdalDataset );
Expand Down Expand Up @@ -2606,7 +2609,6 @@ void QgsGdalProvider::initBaseDataset()
{
QgsLogger::warning( QStringLiteral( "Warped VRT Creation failed." ) );
mGdalDataset = mGdalBaseDataset;
GDALReferenceDataset( mGdalDataset );
}
else
{
Expand All @@ -2616,7 +2618,6 @@ void QgsGdalProvider::initBaseDataset()
else
{
mGdalDataset = mGdalBaseDataset;
GDALReferenceDataset( mGdalDataset );
}

if ( !hasGeoTransform )
Expand Down Expand Up @@ -2645,12 +2646,7 @@ void QgsGdalProvider::initBaseDataset()
{
appendError( ERRMSG( tr( "Cannot get GDAL raster band: %1" ).arg( msg ) ) );

GDALDereferenceDataset( mGdalBaseDataset );
mGdalBaseDataset = nullptr;

GDALClose( mGdalDataset );
mGdalDataset = nullptr;
mValid = false;
closeDataset();
return;
}
// if there are subdatasets, leave the dataset open for subsequent queries
Expand Down

0 comments on commit 651ccb4

Please sign in to comment.