Skip to content
Permalink
Browse files
Fix locking of ogr datasource in case the datasource is shared (trans…
…action mode)
  • Loading branch information
mhugent authored and github-actions committed Nov 18, 2021
1 parent 87717ed commit 0ce2bf5029fc07b603d53c1489a2a5bdbf1b8b54
Showing with 22 additions and 7 deletions.
  1. +17 −6 src/core/providers/ogr/qgsogrfeatureiterator.cpp
  2. +1 −0 src/core/providers/ogr/qgsogrfeatureiterator.h
  3. +4 −1 src/core/providers/ogr/qgsogrproviderutils.h
@@ -53,6 +53,11 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
, mSymbolType( QgsSymbol::symbolTypeForGeometryType( QgsWkbTypes::geometryType( source->mWkbType ) ) )
{

if ( mSharedDS )
{
mTransactionDSLocker = new QMutexLocker( &mSharedDS->sharedDSMutex() );
}

/* When inside a transaction for GPKG/SQLite and fetching fid(s) we might be nested inside an outer fetching loop,
* (see GH #39178) so we need to skip all calls that might reset the reading (rewind) to avoid an endless loop in the
* outer fetching iterator that uses the same connection.
@@ -124,7 +129,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
}
}
}
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
//QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );

if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs )
{
@@ -280,6 +285,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
QgsOgrFeatureIterator::~QgsOgrFeatureIterator()
{
close();
delete mTransactionDSLocker;
}

bool QgsOgrFeatureIterator::nextFeatureFilterExpression( QgsFeature &f )
@@ -372,8 +378,7 @@ void QgsOgrFeatureIterator::setInterruptionChecker( QgsFeedback *interruptionChe

bool QgsOgrFeatureIterator::fetchFeature( QgsFeature &feature )
{
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );

//QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg, mInterruptionChecker );
QgsSetCPLHTTPFetchOverriderInitiatorClass( oCPLHTTPFetcher, QStringLiteral( "QgsOgrFeatureIterator" ) )

@@ -469,8 +474,7 @@ void QgsOgrFeatureIterator::resetReading()

bool QgsOgrFeatureIterator::rewind()
{
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );

//QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
if ( mClosed || !mOgrLayer )
return false;

@@ -487,9 +491,13 @@ bool QgsOgrFeatureIterator::close()
if ( mSharedDS )
{
iteratorClosed();

/*if ( mSharedDS )
{
mSharedDS->sharedDSMutex().unlock();
}*/
mOgrLayer = nullptr;
mSharedDS.reset();

mClosed = true;
return true;
}
@@ -525,6 +533,9 @@ bool QgsOgrFeatureIterator::close()
mOgrLayer = nullptr;

mClosed = true;



return true;
}

@@ -104,6 +104,7 @@ class QgsOgrFeatureIterator final: public QgsAbstractFeatureIteratorFromSource<Q
QgsRectangle mFilterRect;
QgsCoordinateTransform mTransform;
QgsOgrDatasetSharedPtr mSharedDS = nullptr;
QMutexLocker *mTransactionDSLocker = nullptr;

bool mFirstFieldIsFid = false;
QgsFields mFieldsWithoutFid;
@@ -95,14 +95,15 @@ class CORE_EXPORT QgsOgrProviderUtils
#else
QRecursiveMutex mutex;
#endif
QMutex sharedDSMutex;
GDALDatasetH hDS = nullptr;
QMap<QString, QgsOgrLayer *> setLayers;
int refCount = 0;
bool canBeShared = true;

DatasetWithLayers()
#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)
: mutex( QMutex::Recursive )
: mutex( QMutex::Recursive ), sharedDSMutex( QMutex::Recursive )
#endif
{}
};
@@ -304,6 +305,8 @@ class QgsOgrDataset
QRecursiveMutex &mutex() { return mDs->mutex; }
#endif

QMutex &sharedDSMutex() { return mDs->sharedDSMutex; }

bool executeSQLNoReturn( const QString &sql );

OGRLayerH getLayerFromNameOrIndex( const QString &layerName, int layerIndex );

0 comments on commit 0ce2bf5

Please sign in to comment.