[postgres] Avoid deadlocks in transactional editing

m-kuhn · m-kuhn · commit 2e515a2ccc8f · 2016-04-14T09:39:51.000+02:00
Can currently be triggered by using the field calculator to update a selection.
While an iterator is active and the connection is locked, an update statement
waits unsuccessfully for the same (locked) connection.

This commit only locks the connection while an iterator is actually fetching
data and not for its entire lifetime.
diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp
@@ -46,7 +46,6 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
   else
   {
     mConn = source->mTransactionConnection;
-    mConn->lock();
     mIsTransactionConnection = true;
   }
 
@@ -227,6 +226,8 @@ bool QgsPostgresFeatureIterator::fetchFeature( QgsFeature& feature )
   {
     QString fetch = QString( "FETCH FORWARD %1 FROM %2" ).arg( mFeatureQueueSize ).arg( mCursorName );
     QgsDebugMsgLevel( QString( "fetching %1 features." ).arg( mFeatureQueueSize ), 4 );
+
+    lock();
     if ( mConn->PQsendQuery( fetch ) == 0 ) // fetch features asynchronously
     {
       QgsMessageLog::logMessage( QObject::tr( "Fetching from cursor %1 failed\nDatabase error: %2" ).arg( mCursorName, mConn->PQerrorMessage() ), QObject::tr( "PostGIS" ) );
@@ -257,6 +258,7 @@ bool QgsPostgresFeatureIterator::fetchFeature( QgsFeature& feature )
         getFeature( queryResult, row, mFeatureQueue.back() );
       } // for each row in queue
     }
+    unlock();
   }
 
   if ( mFeatureQueue.empty() )
@@ -319,13 +321,28 @@ bool QgsPostgresFeatureIterator::prepareOrderBy( const QList<QgsFeatureRequest::
   return mOrderByCompiled;
 }
 
+void QgsPostgresFeatureIterator::lock()
+{
+  if ( mIsTransactionConnection )
+    mConn->lock();
+}
+
+void QgsPostgresFeatureIterator::unlock()
+{
+  if ( mIsTransactionConnection )
+    mConn->unlock();
+}
+
 bool QgsPostgresFeatureIterator::rewind()
 {
   if ( mClosed )
     return false;
 
   // move cursor to first record
+
+  lock();
   mConn->PQexecNR( QString( "move absolute 0 in %1" ).arg( mCursorName ) );
+  unlock();
   mFeatureQueue.clear();
   mFetched = 0;
   mLastFetch = false;
@@ -338,16 +355,14 @@ bool QgsPostgresFeatureIterator::close()
   if ( !mConn )
     return false;
 
+  lock();
   mConn->closeCursor( mCursorName );
+  unlock();
 
   if ( !mIsTransactionConnection )
   {
     QgsPostgresConnPool::instance()->releaseConnection( mConn );
   }
-  else
-  {
-    mConn->unlock();
-  }
   mConn = nullptr;
 
   while ( !mFeatureQueue.empty() )
@@ -598,15 +613,17 @@ bool QgsPostgresFeatureIterator::declareCursor( const QString& whereClause, long
   if ( !orderBy.isEmpty() )
     query += QString( " 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/qgspostgresfeatureiterator.h b/src/providers/postgres/qgspostgresfeatureiterator.h
@@ -126,6 +126,9 @@ class QgsPostgresFeatureIterator : public QgsAbstractFeatureIteratorFromSource<Q
 
     virtual bool prepareOrderBy( const QList<QgsFeatureRequest::OrderByClause> &orderBys ) override;
 
+    inline void lock();
+    inline void unlock();
+
     bool mExpressionCompiled;
     bool mOrderByCompiled;
     bool mLastFetch;
