Merge pull request #2757 from manisandro/pool_invalidate_release

Connection pool improvements
qgis · Feb 5, 2016 · 8cdd723 · 8cdd723
2 parents 070b28a + 95030d6
commit 8cdd723
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 25 deletions.
diff --git a/src/core/qgsconnectionpool.h b/src/core/qgsconnectionpool.h
@@ -128,15 +128,22 @@ class QgsConnectionPoolGroup
     {
       connMutex.lock();
       acquiredConns.removeAll( conn );
-      Item i;
-      i.c = conn;
-      i.lastUsedTime = QTime::currentTime();
-      conns.push( i );
-
-      if ( !expirationTimer->isActive() )
+      if ( !qgsConnectionPool_ConnectionIsValid( conn ) )
+      {
+        qgsConnectionPool_ConnectionDestroy( conn );
+      }
+      else
       {
-        // will call the slot directly or queue the call (if the object lives in a different thread)
-        QMetaObject::invokeMethod( expirationTimer->parent(), "startExpirationTimer" );
+        Item i;
+        i.c = conn;
+        i.lastUsedTime = QTime::currentTime();
+        conns.push( i );
+
+        if ( !expirationTimer->isActive() )
+        {
+          // will call the slot directly or queue the call (if the object lives in a different thread)
+          QMetaObject::invokeMethod( expirationTimer->parent(), "startExpirationTimer" );
+        }
       }
 
       connMutex.unlock();
@@ -149,8 +156,9 @@ class QgsConnectionPoolGroup
       connMutex.lock();
       Q_FOREACH ( Item i, conns )
       {
-        qgsConnectionPool_InvalidateConnection( i.c );
+        qgsConnectionPool_ConnectionDestroy( i.c );
       }
+      conns.clear();
       Q_FOREACH ( T c, acquiredConns )
         qgsConnectionPool_InvalidateConnection( c );
       connMutex.unlock();

diff --git a/src/providers/ogr/qgsogrconnpool.h b/src/providers/ogr/qgsogrconnpool.h
@@ -105,6 +105,15 @@ class QgsOgrConnPool : public QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup
     //
     static void cleanupInstance();
 
+    /**
+     * @brief Increases the reference count on the connection pool for the specified connection.
+     * @param connInfo The connection string.
+     * @note
+     *     Any user of the connection pool needs to increase the reference count
+     *     before it acquires any connections and decrease the reference count after
+     *     releasing all acquired connections to ensure that all open OGR handles
+     *     are freed when and only when no one is using the pool anymore.
+     */
     void ref( const QString& connInfo )
     {
       mMutex.lock();
@@ -115,6 +124,10 @@ class QgsOgrConnPool : public QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup
       mMutex.unlock();
     }
 
+    /**
+     * @brief Decrease the reference count on the connection pool for the specified connection.
+     * @param connInfo The connection string.
+     */
     void unref( const QString& connInfo )
     {
       mMutex.lock();
@@ -133,16 +146,6 @@ class QgsOgrConnPool : public QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup
       mMutex.unlock();
     }
 
-    static void refS( const QString &connInfo )
-    {
-      instance()->ref( connInfo );
-    }
-
-    static void unrefS( const QString &connInfo )
-    {
-      instance()->unref( connInfo );
-    }
-
   protected:
     Q_DISABLE_COPY( QgsOgrConnPool )
 

diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp
@@ -404,12 +404,12 @@ QgsOgrFeatureSource::QgsOgrFeatureSource( const QgsOgrProvider* p )
   mFields = p->mAttributeFields;
   mDriverName = p->ogrDriverName;
   mOgrGeometryTypeFilter = wkbFlatten( p->mOgrGeometryTypeFilter );
-  QgsOgrConnPool::refS( mFilePath );
+  QgsOgrConnPool::instance()->ref( mFilePath );
 }
 
 QgsOgrFeatureSource::~QgsOgrFeatureSource()
 {
-  QgsOgrConnPool::unrefS( mFilePath );
+  QgsOgrConnPool::instance()->unref( mFilePath );
 }
 
 QgsFeatureIterator QgsOgrFeatureSource::getFeatures( const QgsFeatureRequest& request )

diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -374,7 +374,7 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri )
     << QgsVectorDataProvider::NativeType( tr( "Date & Time" ), "datetime", QVariant::DateTime );
   }
 
-  QgsOgrConnPool::refS( mFilePath );
+  QgsOgrConnPool::instance()->ref( mFilePath );
 }
 
 QgsOgrProvider::~QgsOgrProvider()
@@ -2590,7 +2590,7 @@ QString QgsOgrUtils::quotedValue( const QVariant& value )
 bool QgsOgrProvider::syncToDisc()
 {
   //for shapefiles, remove spatial index files and create a new index
-  QgsOgrConnPool::unrefS( mFilePath );
+  QgsOgrConnPool::instance()->unref( mFilePath );
   bool shapeIndex = false;
   if ( ogrDriverName == "ESRI Shapefile" )
   {
@@ -2621,7 +2621,7 @@ bool QgsOgrProvider::syncToDisc()
 
   mShapefileMayBeCorrupted = false;
 
-  QgsOgrConnPool::refS( mFilePath );
+  QgsOgrConnPool::instance()->ref( mFilePath );
   if ( shapeIndex )
   {
     return createSpatialIndex();
@@ -2834,7 +2834,7 @@ void QgsOgrProvider::close()
 
   updateExtents();
 
-  QgsOgrConnPool::unrefS( mFilePath );
+  QgsOgrConnPool::instance()->unref( mFilePath );
 }
 
 // ---------------------------------------------------------------------------

diff --git a/tests/src/analysis/testqgsvectoranalyzer.cpp b/tests/src/analysis/testqgsvectoranalyzer.cpp
@@ -86,6 +86,9 @@ void  TestQgsVectorAnalyzer::initTestCase()
 }
 void  TestQgsVectorAnalyzer::cleanupTestCase()
 {
+  delete mpLineLayer;
+  delete mpPolyLayer;
+  delete mpPointLayer;
   QgsApplication::exitQgis();
 }
 void  TestQgsVectorAnalyzer::init()