diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp index 8573547722a..8862f65043f 100644 --- a/src/providers/postgres/qgspostgresconn.cpp +++ b/src/providers/postgres/qgspostgresconn.cpp @@ -211,6 +211,7 @@ QgsPostgresConn::QgsPostgresConn( const QString &conninfo, bool readOnly, bool s , mNextCursorId( 0 ) , mShared( shared ) , mTransaction( transaction ) + , mLock( QMutex::Recursive ) { QgsDebugMsg( QStringLiteral( "New PostgreSQL connection for " ) + conninfo ); @@ -1065,33 +1066,37 @@ PGresult *QgsPostgresConn::PQexec( const QString &query, bool logError ) const } QgsDebugMsgLevel( QStringLiteral( "Executing SQL: %1" ).arg( query ), 3 ); - PGresult *res = ::PQexec( mConn, query.toUtf8() ); - - if ( res ) + PGresult *res = nullptr; { - int errorStatus = PQresultStatus( res ); - if ( errorStatus != PGRES_COMMAND_OK && errorStatus != PGRES_TUPLES_OK ) + QMutexLocker locker( &mLock ); + res = ::PQexec( mConn, query.toUtf8() ); + + if ( res ) { - if ( logError ) + int errorStatus = PQresultStatus( res ); + if ( errorStatus != PGRES_COMMAND_OK && errorStatus != PGRES_TUPLES_OK ) { - QgsMessageLog::logMessage( tr( "Erroneous query: %1 returned %2 [%3]" ) - .arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ), - tr( "PostGIS" ) ); - } - else - { - QgsDebugMsg( QStringLiteral( "Not logged erroneous query: %1 returned %2 [%3]" ) - .arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ) ); + if ( logError ) + { + QgsMessageLog::logMessage( tr( "Erroneous query: %1 returned %2 [%3]" ) + .arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ), + tr( "PostGIS" ) ); + } + else + { + QgsDebugMsg( QStringLiteral( "Not logged erroneous query: %1 returned %2 [%3]" ) + .arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ) ); + } } } - } - else if ( logError ) - { - QgsMessageLog::logMessage( tr( "Query failed: %1\nError: no result buffer" ).arg( query ), tr( "PostGIS" ) ); - } - else - { - QgsDebugMsg( QStringLiteral( "Not logged query failed: %1\nError: no result buffer" ).arg( query ) ); + else if ( logError ) + { + QgsMessageLog::logMessage( tr( "Query failed: %1\nError: no result buffer" ).arg( query ), tr( "PostGIS" ) ); + } + else + { + QgsDebugMsg( QStringLiteral( "Not logged query failed: %1\nError: no result buffer" ).arg( query ) ); + } } return res; @@ -1099,6 +1104,8 @@ PGresult *QgsPostgresConn::PQexec( const QString &query, bool logError ) const bool QgsPostgresConn::openCursor( const QString &cursorName, const QString &sql ) { + QMutexLocker locker( &mLock ); // to protect access to mOpenCursors + if ( mOpenCursors++ == 0 && !mTransaction ) { QgsDebugMsgLevel( QStringLiteral( "Starting read-only transaction: %1" ).arg( mPostgresqlVersion ), 4 ); @@ -1114,6 +1121,8 @@ bool QgsPostgresConn::openCursor( const QString &cursorName, const QString &sql bool QgsPostgresConn::closeCursor( const QString &cursorName ) { + QMutexLocker locker( &mLock ); // to protect access to mOpenCursors + if ( !PQexecNR( QStringLiteral( "CLOSE %1" ).arg( cursorName ) ) ) return false; @@ -1128,11 +1137,14 @@ bool QgsPostgresConn::closeCursor( const QString &cursorName ) QString QgsPostgresConn::uniqueCursorName() { + QMutexLocker locker( &mLock ); // to protect access to mNextCursorId return QStringLiteral( "qgis_%1" ).arg( ++mNextCursorId ); } bool QgsPostgresConn::PQexecNR( const QString &query, bool retry ) { + QMutexLocker locker( &mLock ); // to protect access to mOpenCursors + QgsPostgresResult res( PQexec( query, false ) ); ExecStatusType errorStatus = res.PQresultStatus(); @@ -1194,11 +1206,15 @@ PGresult *QgsPostgresConn::PQgetResult() PGresult *QgsPostgresConn::PQprepare( const QString &stmtName, const QString &query, int nParams, const Oid *paramTypes ) { + QMutexLocker locker( &mLock ); + return ::PQprepare( mConn, stmtName.toUtf8(), query.toUtf8(), nParams, paramTypes ); } PGresult *QgsPostgresConn::PQexecPrepared( const QString &stmtName, const QStringList ¶ms ) { + QMutexLocker locker( &mLock ); + const char **param = new const char *[ params.size()]; QList qparam; @@ -1222,6 +1238,8 @@ PGresult *QgsPostgresConn::PQexecPrepared( const QString &stmtName, const QStrin void QgsPostgresConn::PQfinish() { + QMutexLocker locker( &mLock ); + Q_ASSERT( mConn ); ::PQfinish( mConn ); mConn = nullptr; @@ -1229,12 +1247,16 @@ void QgsPostgresConn::PQfinish() int QgsPostgresConn::PQstatus() const { + QMutexLocker locker( &mLock ); + Q_ASSERT( mConn ); return ::PQstatus( mConn ); } QString QgsPostgresConn::PQerrorMessage() const { + QMutexLocker locker( &mLock ); + Q_ASSERT( mConn ); return QString::fromUtf8( ::PQerrorMessage( mConn ) ); } diff --git a/src/providers/postgres/qgspostgresconn.h b/src/providers/postgres/qgspostgresconn.h index 2b82550b4f5..1f95a477b69 100644 --- a/src/providers/postgres/qgspostgresconn.h +++ b/src/providers/postgres/qgspostgresconn.h @@ -244,16 +244,26 @@ class QgsPostgresConn : public QObject // libpq wrapper // - // run a query and check for errors + // run a query and check for errors, thread-safe PGresult *PQexec( const QString &query, bool logError = true ) const; void PQfinish(); QString PQerrorMessage() const; - int PQsendQuery( const QString &query ); int PQstatus() const; - PGresult *PQgetResult(); PGresult *PQprepare( const QString &stmtName, const QString &query, int nParams, const Oid *paramTypes ); PGresult *PQexecPrepared( const QString &stmtName, const QStringList ¶ms ); + /** + * PQsendQuery is used for asynchronous queries (with PQgetResult) + * Thread safety must be ensured by the caller by calling QgsPostgresConn::lock() and QgsPostgresConn::unlock() + */ + int PQsendQuery( const QString &query ); + + /** + * PQgetResult is used for asynchronous queries (with PQsendQuery) + * Thread safety must be ensured by the caller by calling QgsPostgresConn::lock() and QgsPostgresConn::unlock() + */ + PGresult *PQgetResult(); + bool begin(); bool commit(); bool rollback(); @@ -425,7 +435,7 @@ class QgsPostgresConn : public QObject bool mTransaction; - QMutex mLock; + mutable QMutex mLock; }; // clazy:excludeall=qstring-allocations diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp index 8229e3ced47..4b433b7348d 100644 --- a/src/providers/postgres/qgspostgresfeatureiterator.cpp +++ b/src/providers/postgres/qgspostgresfeatureiterator.cpp @@ -386,9 +386,7 @@ bool QgsPostgresFeatureIterator::rewind() // move cursor to first record - lock(); mConn->PQexecNR( QStringLiteral( "move absolute 0 in %1" ).arg( mCursorName ) ); - unlock(); mFeatureQueue.clear(); mFetched = 0; mLastFetch = false; @@ -401,9 +399,7 @@ bool QgsPostgresFeatureIterator::close() if ( !mConn ) return false; - lock(); mConn->closeCursor( mCursorName ); - unlock(); if ( !mIsTransactionConnection ) { @@ -651,17 +647,14 @@ bool QgsPostgresFeatureIterator::declareCursor( const QString &whereClause, long if ( !orderBy.isEmpty() ) query += QStringLiteral( " ORDER BY %1 " ).arg( orderBy ); - lock(); if ( !mConn->openCursor( mCursorName, query ) ) { - unlock(); // reloading the fields might help next time around // TODO how to cleanly force reload of fields? P->loadFields(); if ( closeOnFail ) close(); return false; } - unlock(); mLastFetch = false; return true; diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp index 540c01d859b..f68126cb644 100644 --- a/src/providers/postgres/qgspostgresprovider.cpp +++ b/src/providers/postgres/qgspostgresprovider.cpp @@ -4095,9 +4095,7 @@ QgsCoordinateReferenceSystem QgsPostgresProvider::crs() const else { QgsPostgresConn *conn = connectionRO(); - conn->lock(); QgsPostgresResult result( conn->PQexec( QStringLiteral( "SELECT proj4text FROM spatial_ref_sys WHERE srid=%1" ).arg( srid ) ) ); - conn->unlock(); if ( result.PQresultStatus() == PGRES_TUPLES_OK ) { srs = QgsCoordinateReferenceSystem::fromProj4( result.PQgetvalue( 0, 0 ) ); diff --git a/src/providers/postgres/qgspostgrestransaction.cpp b/src/providers/postgres/qgspostgrestransaction.cpp index d8584db6eb5..dfb404ca5ea 100644 --- a/src/providers/postgres/qgspostgrestransaction.cpp +++ b/src/providers/postgres/qgspostgrestransaction.cpp @@ -71,9 +71,7 @@ bool QgsPostgresTransaction::executeSql( const QString &sql, QString &errorMsg, } QgsDebugMsg( QStringLiteral( "Transaction sql: %1" ).arg( sql ) ); - mConn->lock(); QgsPostgresResult r( mConn->PQexec( sql, true ) ); - mConn->unlock(); if ( r.PQresultStatus() == PGRES_BAD_RESPONSE || r.PQresultStatus() == PGRES_FATAL_ERROR ) {