Ensure GDAL deinitialization runs after last possible use

Sandro Santilli · Sandro Santilli · commit 051253888810 · 2016-02-02T17:24:06.000+01:00
Closes #14176
diff --git a/src/core/qgsapplication.cpp b/src/core/qgsapplication.cpp
@@ -876,6 +876,10 @@ void QgsApplication::exitQgis()
 
   //delete all registered functions from expression engine (see above comment)
   QgsExpression::cleanRegisteredFunctions();
+
+  // tear-down GDAL/OGR
+  OGRCleanupAll();
+  GDALDestroyDriverManager();
 }
 
 QString QgsApplication::showSettings()
diff --git a/src/providers/gdal/qgsgdalprovider.cpp b/src/providers/gdal/qgsgdalprovider.cpp
@@ -3021,5 +3021,6 @@ QGISEXTERN QList<QPair<QString, QString> > *pyramidResamplingMethods()
 
 QGISEXTERN void cleanupProvider()
 {
-  GDALDestroyDriverManager();
+  // nothing to do here, QgsApplication takes care of
+  // calling GDALDestroyDriverManager()
 }
diff --git a/src/providers/ogr/qgsogrconnpool.cpp b/src/providers/ogr/qgsogrconnpool.cpp
@@ -15,12 +15,20 @@
 
 #include "qgsogrconnpool.h"
 
-QgsOgrConnPool QgsOgrConnPool::sInstance;
-bool QgsOgrConnPool::sInstanceDestroyed = false;
+QgsOgrConnPool* QgsOgrConnPool::mInstance = nullptr;
 
+// static public
 QgsOgrConnPool* QgsOgrConnPool::instance()
 {
-  return sInstanceDestroyed ? nullptr : &sInstance;
+  if ( ! mInstance ) mInstance = new QgsOgrConnPool();
+  return mInstance;
+}
+
+// static public
+void QgsOgrConnPool::cleanupInstance()
+{
+  delete mInstance;
+  mInstance = nullptr;
 }
 
 QgsOgrConnPool::QgsOgrConnPool() : QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup>()
@@ -31,5 +39,4 @@ QgsOgrConnPool::QgsOgrConnPool() : QgsConnectionPool<QgsOgrConn*, QgsOgrConnPool
 QgsOgrConnPool::~QgsOgrConnPool()
 {
   QgsDebugCall;
-  sInstanceDestroyed = true;
 }
diff --git a/src/providers/ogr/qgsogrconnpool.h b/src/providers/ogr/qgsogrconnpool.h
@@ -86,8 +86,25 @@ class QgsOgrConnPoolGroup : public QObject, public QgsConnectionPoolGroup<QgsOgr
 class QgsOgrConnPool : public QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup>
 {
   public:
+
+    // NOTE: first call to this function initializes the
+    //       singleton.
+    // WARNING: concurrent call from multiple threads may result
+    //          in multiple instances being created, and memory
+    //          leaking at exit.
+    //
     static QgsOgrConnPool* instance();
 
+    // Singleton cleanup
+    //
+    // Make sure nobody is using the instance before calling
+    // this function.
+    //
+    // WARNING: concurrent call from multiple threads may result
+    //          in double-free of the instance.
+    //
+    static void cleanupInstance();
+
     void ref( const QString& connInfo )
     {
       mMutex.lock();
@@ -118,14 +135,12 @@ class QgsOgrConnPool : public QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup
 
     static void refS( const QString &connInfo )
     {
-      if ( instance() )
-        instance()->ref( connInfo );
+      instance()->ref( connInfo );
     }
 
     static void unrefS( const QString &connInfo )
     {
-      if ( instance() )
-        instance()->unref( connInfo );
+      instance()->unref( connInfo );
     }
 
   protected:
@@ -134,9 +149,7 @@ class QgsOgrConnPool : public QgsConnectionPool<QgsOgrConn*, QgsOgrConnPoolGroup
   private:
     QgsOgrConnPool();
     ~QgsOgrConnPool();
-
-    static QgsOgrConnPool sInstance;
-    static bool sInstanceDestroyed;
+    static QgsOgrConnPool *mInstance;
 };
 
 
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -2857,5 +2857,7 @@ QGISEXTERN QgsVectorLayerImport::ImportError createEmptyLayer(
 
 QGISEXTERN void cleanupProvider()
 {
-  OGRCleanupAll();
+  QgsOgrConnPool::cleanupInstance();
+  // NOTE: QgsApplication takes care of
+  // calling OGRCleanupAll();
 }

Original file line number	Diff line number	Diff line change
`@@ -876,6 +876,10 @@ void QgsApplication::exitQgis()`
`876`	`876`
`877`	`877`	`//delete all registered functions from expression engine (see above comment)`
`878`	`878`	`QgsExpression::cleanRegisteredFunctions();`
	`879`	`+`
	`880`	`+ // tear-down GDAL/OGR`
	`881`	`+ OGRCleanupAll();`
	`882`	`+ GDALDestroyDriverManager();`
`879`	`883`	`}`
`880`	`884`
`881`	`885`	`QString QgsApplication::showSettings()`
Original file line number	Diff line number	Diff line change
`@@ -3021,5 +3021,6 @@ QGISEXTERN QList<QPair<QString, QString> > *pyramidResamplingMethods()`
`3021`	`3021`
`3022`	`3022`	`QGISEXTERN void cleanupProvider()`
`3023`	`3023`	`{`
`3024`		`- GDALDestroyDriverManager();`
	`3024`	`+ // nothing to do here, QgsApplication takes care of`
	`3025`	`+ // calling GDALDestroyDriverManager()`
`3025`	`3026`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2857,5 +2857,7 @@ QGISEXTERN QgsVectorLayerImport::ImportError createEmptyLayer(`
`2857`	`2857`
`2858`	`2858`	`QGISEXTERN void cleanupProvider()`
`2859`	`2859`	`{`
`2860`		`- OGRCleanupAll();`
	`2860`	`+ QgsOgrConnPool::cleanupInstance();`
	`2861`	`+ // NOTE: QgsApplication takes care of`
	`2862`	`+ // calling OGRCleanupAll();`
`2861`	`2863`	`}`