Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix locking of ogr datasource in case the datasource is shared (trans…
…action mode)
  • Loading branch information
mhugent committed Nov 15, 2021
1 parent 546edc6 commit c249b92
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
23 changes: 17 additions & 6 deletions src/core/providers/ogr/qgsogrfeatureiterator.cpp
Expand Up @@ -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.
Expand Down Expand Up @@ -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 )
{
Expand Down Expand Up @@ -280,6 +285,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
QgsOgrFeatureIterator::~QgsOgrFeatureIterator()
{
close();
delete mTransactionDSLocker;
}

bool QgsOgrFeatureIterator::nextFeatureFilterExpression( QgsFeature &f )
Expand Down Expand Up @@ -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" ) )

Expand Down Expand Up @@ -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;

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

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

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

mClosed = true;



return true;
}

Expand Down
1 change: 1 addition & 0 deletions src/core/providers/ogr/qgsogrfeatureiterator.h
Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion src/core/providers/ogr/qgsogrproviderutils.h
Expand Up @@ -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
{}
};
Expand Down Expand Up @@ -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 );
Expand Down

0 comments on commit c249b92

Please sign in to comment.