Fix #10655 (race condition in QgsCredentials)

wonder-sk · wonder-sk · commit 2a4684a28d4c · 2014-06-20T17:20:57.000+07:00
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 -&gt; will request credentials in dialog
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 )

Original file line number	Diff line number	Diff line change
`@@ -80,6 +80,8 @@ QgsOracleConn::QgsOracleConn( QgsDataSourceURI uri )`
`80`	`80`	`if ( !username.isEmpty() )`
`81`	`81`	`realm.prepend( username + "@" );`
`82`	`82`
	`83`	`+ QgsCredentials::instance()->lock();`
	`84`	`+`
`83`	`85`	`while ( !mDatabase.open() )`
`84`	`86`	`{`
`85`	`87`	`bool ok = QgsCredentials::instance()->get( realm, username, password, mDatabase.lastError().text() );`
`@@ -102,6 +104,8 @@ QgsOracleConn::QgsOracleConn( QgsDataSourceURI uri )`
`102`	`104`
`103`	`105`	`if ( mDatabase.isOpen() )`
`104`	`106`	`QgsCredentials::instance()->put( realm, username, password );`
	`107`	`+`
	`108`	`+ QgsCredentials::instance()->unlock();`
`105`	`109`	`}`
`106`	`110`
`107`	`111`	`if ( !mDatabase.isOpen() )`
Original file line number	Diff line number	Diff line change
`@@ -181,6 +181,8 @@ QgsPostgresConn::QgsPostgresConn( QString conninfo, bool readOnly, bool shared )`
`181`	`181`	`QString username = uri.username();`
`182`	`182`	`QString password = uri.password();`
`183`	`183`
	`184`	`+ QgsCredentials::instance()->lock();`
	`185`	`+`
`184`	`186`	`while ( PQstatus() != CONNECTION_OK )`
`185`	`187`	`{`
`186`	`188`	`bool ok = QgsCredentials::instance()->get( conninfo, username, password, PQerrorMessage() );`
`@@ -201,6 +203,8 @@ QgsPostgresConn::QgsPostgresConn( QString conninfo, bool readOnly, bool shared )`
`201`	`203`
`202`	`204`	`if ( PQstatus() == CONNECTION_OK )`
`203`	`205`	`QgsCredentials::instance()->put( conninfo, username, password );`
	`206`	`+`
	`207`	`+ QgsCredentials::instance()->unlock();`
`204`	`208`	`}`
`205`	`209`
`206`	`210`	`if ( PQstatus() != CONNECTION_OK )`