[GDAL provider] Revise how referencing counting is done on mGdalBaseD…

…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.
qgis · Oct 14, 2018 · 651ccb4 · 651ccb4
1 parent e7d15b6
commit 651ccb4
Showing 1 changed file with 8 additions and 12 deletions.
diff --git a/src/providers/gdal/qgsgdalprovider.cpp b/src/providers/gdal/qgsgdalprovider.cpp
@@ -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 );
         }
@@ -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 );
       }
@@ -464,7 +464,7 @@ QgsGdalProvider::~QgsGdalProvider()
     }
     else
     {
-      if ( mGdalBaseDataset )
+      if ( mGdalBaseDataset != mGdalDataset )
       {
         GDALDereferenceDataset( mGdalBaseDataset );
       }
@@ -498,7 +498,10 @@ void QgsGdalProvider::closeDataset()
   }
   mValid = false;
 
-  GDALDereferenceDataset( mGdalBaseDataset );
+  if ( mGdalBaseDataset != mGdalDataset )
+  {
+    GDALDereferenceDataset( mGdalBaseDataset );
+  }
   mGdalBaseDataset = nullptr;
 
   GDALClose( mGdalDataset );
@@ -2606,7 +2609,6 @@ void QgsGdalProvider::initBaseDataset()
     {
       QgsLogger::warning( QStringLiteral( "Warped VRT Creation failed." ) );
       mGdalDataset = mGdalBaseDataset;
-      GDALReferenceDataset( mGdalDataset );
     }
     else
     {
@@ -2616,7 +2618,6 @@ void QgsGdalProvider::initBaseDataset()
   else
   {
     mGdalDataset = mGdalBaseDataset;
-    GDALReferenceDataset( mGdalDataset );
   }
 
   if ( !hasGeoTransform )
@@ -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