[ogr] Fix ref/unref mismatch when loading OGR layers

Causes an extra connection reference which is never removed, blocking ogr dataset closing. Fixes #18420, probably others (cherry-picked from f3b5838)
qgis · Mar 16, 2018 · dff31a4 · dff31a4
1 parent 7521a82
commit dff31a4
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 5 deletions.
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -1686,7 +1686,7 @@ bool QgsOgrProvider::commitTransaction()
   return true;
 }
 
-bool QgsOgrProvider::_setSubsetString( const QString &theSQL, bool updateFeatureCount, bool updateCapabilities )
+bool QgsOgrProvider::_setSubsetString( const QString &theSQL, bool updateFeatureCount, bool updateCapabilities, bool hasExistingRef )
 {
   QgsCPLErrorHandler handler;
 
@@ -1745,9 +1745,11 @@ bool QgsOgrProvider::_setSubsetString( const QString &theSQL, bool updateFeature
 
   if ( uri != dataSourceUri() )
   {
-    QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
+    if ( hasExistingRef )
+      QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
     setDataSourceUri( uri );
-    QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
+    if ( hasExistingRef )
+      QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
   }
 
   mOgrLayer->ResetReading();
@@ -3994,8 +3996,13 @@ void QgsOgrProvider::open( OpenMode mode )
     mSubsetString.clear();
     // Block signals to avoid endless recusion reloadData -> emit dataChanged -> reloadData
     blockSignals( true );
+
     // Do not update capabilities: it will be done later
-    mValid = _setSubsetString( origSubsetString, true, false );
+
+    // WARNING if this is the initial open - we don't already have a connection ref, and will be creating one later. So we *mustn't* grab an extra connection ref
+    // while setting the subset string, or we'll be left with an extra reference which is never cleared.
+    mValid = _setSubsetString( origSubsetString, true, false, mode != OpenModeInitial );
+
     blockSignals( false );
     if ( mValid )
     {

diff --git a/src/providers/ogr/qgsogrprovider.h b/src/providers/ogr/qgsogrprovider.h
@@ -211,7 +211,7 @@ class QgsOgrProvider : public QgsVectorDataProvider
     bool commitTransaction();
 
     //! Does the real job of settings the subset string and adds an argument to disable update capabilities
-    bool _setSubsetString( const QString &theSQL, bool updateFeatureCount = true, bool updateCapabilities = true );
+    bool _setSubsetString( const QString &theSQL, bool updateFeatureCount = true, bool updateCapabilities = true, bool hasExistingRef = true );
 
     void addSubLayerDetailsToSubLayerList( int i, QgsOgrLayer *layer ) const;