Skip to content
Permalink
Browse files

[mssql] Fix database connection cleanup on thread exit

  • Loading branch information
nyalldawson committed Oct 31, 2018
1 parent a392a16 commit a5ff6db24066642e2b5952b22bfe069013ec9a0b
Showing with 32 additions and 1 deletion.
  1. +29 −1 src/providers/mssql/qgsmssqlconnection.cpp
  2. +3 −0 src/providers/mssql/qgsmssqlconnection.h
@@ -24,8 +24,10 @@
#include <QSqlError>
#include <QSqlQuery>
#include <QSet>
#include <QCoreApplication>

int QgsMssqlConnection::sConnectionId = 0;
QMutex QgsMssqlConnection::sMutex{ QMutex::Recursive };

QSqlDatabase QgsMssqlConnection::getDatabase( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password )
{
@@ -50,13 +52,39 @@ QSqlDatabase QgsMssqlConnection::getDatabase( const QString &service, const QStr
connectionName = service;

const QString threadSafeConnectionName = dbConnectionName( connectionName );

// 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 ( !QSqlDatabase::contains( threadSafeConnectionName ) )
{
db = QSqlDatabase::addDatabase( QStringLiteral( "QODBC" ), 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, QCoreApplication::instance(), [connectionName]
{
QMutexLocker locker( &sMutex );
QSqlDatabase::removeDatabase( connectionName );
}, Qt::DirectConnection );
}
}
else
{
db = QSqlDatabase::database( threadSafeConnectionName );
}
locker.unlock();

db.setHostName( host );
QString connectionString;
@@ -306,5 +334,5 @@ QString QgsMssqlConnection::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.
return QStringLiteral( "%1:0x%2" ).arg( name, QString::number( reinterpret_cast< quintptr >( QThread::currentThread() ), 16 ) );
return QStringLiteral( "%1:0x%2" ).arg( name ).arg( reinterpret_cast<quintptr>( QThread::currentThread() ), 2 * QT_POINTER_SIZE, 16, QLatin1Char( '0' ) );
}
@@ -19,6 +19,7 @@
#define QGSMSSQLCONNECTION_H

#include <QStringList>
#include <QMutex>

class QString;
class QSqlDatabase;
@@ -153,6 +154,8 @@ class QgsMssqlConnection
static QString dbConnectionName( const QString &name );

static int sConnectionId;

static QMutex sMutex;
};

#endif // QGSMSSQLCONNECTION_H

0 comments on commit a5ff6db

Please sign in to comment.
You can’t perform that action at this time.