Fix #10655 (race condition in QgsCredentials)

Example of race condition during rendering: Threads 1 and 2 call get(), it checks that there are cached credentials. Thread 1 takes the cached credentials, thread 2 will get no data -> will request credentials in dialog
qgis · Jun 20, 2014 · 2a4684a · 2a4684a · 3nids · Jun 20, 2014
1 parent 61e934b
commit 2a4684a
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 0 deletions.
diff --git a/python/core/qgscredentials.sip b/python/core/qgscredentials.sip
@@ -14,12 +14,30 @@ class QgsCredentials
     //! retrieves instance
     static QgsCredentials *instance();
 
+    /**
+     * Lock the instance against access from multiple threads. This does not really lock access to get/put methds,
+     * it will just prevent other threads to lock the instance and continue the execution. When the class is used
+     * from non-GUI threads, they should call lock() before the get/put calls to avoid race conditions.
+     * @note added in 2.4
+     */
+    void lock();
+    /**
+     * Unlock the instance after being locked.
+     * @note added in 2.4
+     */
+    void unlock();
+
   protected:
+    QgsCredentials();
+
     //! request a password
     virtual bool request( QString realm, QString &username /In,Out/, QString &password /In,Out/, QString message = QString::null ) = 0;
 
     //! register instance
     void setInstance( QgsCredentials *theInstance );
+
+  private:
+    QgsCredentials( const QgsCredentials& );
 };
 
 

diff --git a/src/core/qgscredentials.cpp b/src/core/qgscredentials.cpp
@@ -38,6 +38,10 @@ QgsCredentials *QgsCredentials::instance()
   return new QgsCredentialsConsole();
 }
 
+QgsCredentials::QgsCredentials()
+{
+}
+
 QgsCredentials::~QgsCredentials()
 {
 }
@@ -70,6 +74,18 @@ void QgsCredentials::put( QString realm, QString username, QString password )
   mCredentialCache.insert( realm, QPair<QString, QString>( username, password ) );
 }
 
+
+void QgsCredentials::lock()
+{
+  mMutex.lock();
+}
+
+void QgsCredentials::unlock()
+{
+  mMutex.unlock();
+}
+
+
 ////////////////////////////////
 // QgsCredentialsConsole
 

diff --git a/src/core/qgscredentials.h b/src/core/qgscredentials.h
@@ -21,6 +21,7 @@
 #include <QObject>
 #include <QPair>
 #include <QMap>
+#include <QMutex>
 
 /** \ingroup core
  * Interface for requesting credentials in QGIS in GUI independent way.
@@ -45,19 +46,38 @@ class CORE_EXPORT QgsCredentials
     //! retrieves instance
     static QgsCredentials *instance();
 
+    /**
+     * Lock the instance against access from multiple threads. This does not really lock access to get/put methds,
+     * it will just prevent other threads to lock the instance and continue the execution. When the class is used
+     * from non-GUI threads, they should call lock() before the get/put calls to avoid race conditions.
+     * @note added in 2.4
+     */
+    void lock();
+    /**
+     * Unlock the instance after being locked.
+     * @note added in 2.4
+     */
+    void unlock();
+
   protected:
+    QgsCredentials();
+
     //! request a password
     virtual bool request( QString realm, QString &username, QString &password, QString message = QString::null ) = 0;
 
     //! register instance
     void setInstance( QgsCredentials *theInstance );
 
   private:
+    Q_DISABLE_COPY( QgsCredentials )
+
     //! cache for already requested credentials in this session
     QMap< QString, QPair<QString, QString> > mCredentialCache;
 
     //! Pointer to the credential instance
     static QgsCredentials *smInstance;
+
+    QMutex mMutex;
 };
 
 

diff --git a/src/providers/oracle/qgsoracleconn.cpp b/src/providers/oracle/qgsoracleconn.cpp
@@ -80,6 +80,8 @@ QgsOracleConn::QgsOracleConn( QgsDataSourceURI uri )
     if ( !username.isEmpty() )
       realm.prepend( username + "@" );
 
+    QgsCredentials::instance()->lock();
+
     while ( !mDatabase.open() )
     {
       bool ok = QgsCredentials::instance()->get( realm, username, password, mDatabase.lastError().text() );
@@ -102,6 +104,8 @@ QgsOracleConn::QgsOracleConn( QgsDataSourceURI uri )
 
     if ( mDatabase.isOpen() )
       QgsCredentials::instance()->put( realm, username, password );
+
+    QgsCredentials::instance()->unlock();
   }
 
   if ( !mDatabase.isOpen() )

diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp
@@ -181,6 +181,8 @@ QgsPostgresConn::QgsPostgresConn( QString conninfo, bool readOnly, bool shared )
     QString username = uri.username();
     QString password = uri.password();
 
+    QgsCredentials::instance()->lock();
+
     while ( PQstatus() != CONNECTION_OK )
     {
       bool ok = QgsCredentials::instance()->get( conninfo, username, password, PQerrorMessage() );
@@ -201,6 +203,8 @@ QgsPostgresConn::QgsPostgresConn( QString conninfo, bool readOnly, bool shared )
 
     if ( PQstatus() == CONNECTION_OK )
       QgsCredentials::instance()->put( conninfo, username, password );
+
+    QgsCredentials::instance()->unlock();
   }
 
   if ( PQstatus() != CONNECTION_OK )