Update src/providers/db2/qgsdb2provider.cpp

dwadler · nyalldawson · commit dd2ed9cd301d · 2018-11-20T11:03:55.000+11:00
diff --git a/src/providers/db2/qgsdb2provider.cpp b/src/providers/db2/qgsdb2provider.cpp
@@ -34,6 +34,7 @@ static const QString PROVIDER_KEY = QStringLiteral( "DB2" );
 static const QString PROVIDER_DESCRIPTION = QStringLiteral( "DB2 Spatial Extender provider" );
 
 int QgsDb2Provider::sConnectionId = 0;
+QMutex QgsDb2Provider::sMutex{ QMutex::Recursive };
 
 QgsDb2Provider::QgsDb2Provider( const QString &uri, const ProviderOptions &options )
   : QgsVectorDataProvider( uri, options )
@@ -193,17 +194,42 @@ QSqlDatabase QgsDb2Provider::getDatabase( const QString &connInfo, QString &errM
   const QString threadSafeConnectionName = dbConnectionName( connectionName );
   QgsDebugMsg( "threadSafeConnectionName: " + threadSafeConnectionName );
 
+  // while everything we use from QSqlDatabase here is thread safe, we need to ensure
+  // that the connection cleanup on thread finalization happens in a predictable order
+  QMutexLocker locker( &sMutex );
+
   /* if new database connection */
   if ( !QSqlDatabase::contains( threadSafeConnectionName ) )
   {
     QgsDebugMsg( QStringLiteral( "new connection. create new QODBC mapping" ) );
     db = QSqlDatabase::addDatabase( QStringLiteral( "QODBC3" ), threadSafeConnectionName );
-  }
+    db.setConnectOptions( QStringLiteral( "SQL_ATTR_CONNECTION_POOLING=SQL_CP_ONE_PER_HENV" ) );
+
+    // for background threads, remove database when current thread finishes
+    if ( QThread::currentThread() != QCoreApplication::instance()->thread() )
+    {
+      QgsDebugMsgLevel( QStringLiteral( "Scheduled auth db remove on thread close" ), 2 );
+
+      // IMPORTANT - we use a direct connection here, because the database removal must happen immediately
+      // when the thread finishes, and we cannot let this get queued on the main thread's event loop.
+      // Otherwise, the QSqlDatabase's private data's thread gets reset immediately the QThread::finished,
+      // and a subsequent call to QSqlDatabase::database with the same thread address (yep it happens, actually a lot)
+      // triggers a condition in QSqlDatabase which detects the nullptr private thread data and returns an invalid database instead.
+      // QSqlDatabase::removeDatabase is thread safe, so this is ok to do.
+      QObject::connect( QThread::currentThread(), &QThread::finished, QThread::currentThread(), [connectionName]
+      {
+        QMutexLocker locker( &sMutex );
+        QSqlDatabase::removeDatabase( connectionName );
+      }, Qt::DirectConnection );
+    }
+	}
   else  /* if existing database connection */
   {
     QgsDebugMsg( QStringLiteral( "found existing connection, use the existing one" ) );
     db = QSqlDatabase::database( threadSafeConnectionName );
   }
+  locker.unlock();  
+  
   db.setHostName( host );
   db.setPort( port.toInt() );
   bool connected = false;
@@ -1759,8 +1785,7 @@ QString QgsDb2Provider::dbConnectionName( const QString &name )
   // Starting with Qt 5.11, sharing the same connection between threads is not allowed.
   // We use a dedicated connection for each thread requiring access to the database,
   // using the thread address as connection name.
-  const QString threadAddress = QStringLiteral( ":0x%1" ).arg( QString::number( reinterpret_cast< quintptr >( QThread::currentThread() ), 16 ) );
-  return name + threadAddress;
+  return QStringLiteral( "%1:0x%2" ).arg( name ).arg( reinterpret_cast<quintptr>( QThread::currentThread() ), 2 * QT_POINTER_SIZE, 16, QLatin1Char( '0' ) );
 }
 
 #ifdef HAVE_GUI
