Skip to content

Commit c249b92

Browse files
committedNov 15, 2021
Fix locking of ogr datasource in case the datasource is shared (transaction mode)
1 parent 546edc6 commit c249b92

File tree

3 files changed

+22
-7
lines changed

3 files changed

+22
-7
lines changed
 

‎src/core/providers/ogr/qgsogrfeatureiterator.cpp

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
5353
, mSymbolType( QgsSymbol::symbolTypeForGeometryType( QgsWkbTypes::geometryType( source->mWkbType ) ) )
5454
{
5555

56+
if ( mSharedDS )
57+
{
58+
mTransactionDSLocker = new QMutexLocker( &mSharedDS->sharedDSMutex() );
59+
}
60+
5661
/* When inside a transaction for GPKG/SQLite and fetching fid(s) we might be nested inside an outer fetching loop,
5762
* (see GH #39178) so we need to skip all calls that might reset the reading (rewind) to avoid an endless loop in the
5863
* outer fetching iterator that uses the same connection.
@@ -124,7 +129,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
124129
}
125130
}
126131
}
127-
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
132+
//QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
128133

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

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

373379
bool QgsOgrFeatureIterator::fetchFeature( QgsFeature &feature )
374380
{
375-
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
376-
381+
//QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
377382
QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg, mInterruptionChecker );
378383
QgsSetCPLHTTPFetchOverriderInitiatorClass( oCPLHTTPFetcher, QStringLiteral( "QgsOgrFeatureIterator" ) )
379384

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

470475
bool QgsOgrFeatureIterator::rewind()
471476
{
472-
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
473-
477+
//QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );
474478
if ( mClosed || !mOgrLayer )
475479
return false;
476480

@@ -487,9 +491,13 @@ bool QgsOgrFeatureIterator::close()
487491
if ( mSharedDS )
488492
{
489493
iteratorClosed();
490-
494+
/*if ( mSharedDS )
495+
{
496+
mSharedDS->sharedDSMutex().unlock();
497+
}*/
491498
mOgrLayer = nullptr;
492499
mSharedDS.reset();
500+
493501
mClosed = true;
494502
return true;
495503
}
@@ -525,6 +533,9 @@ bool QgsOgrFeatureIterator::close()
525533
mOgrLayer = nullptr;
526534

527535
mClosed = true;
536+
537+
538+
528539
return true;
529540
}
530541

‎src/core/providers/ogr/qgsogrfeatureiterator.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ class QgsOgrFeatureIterator final: public QgsAbstractFeatureIteratorFromSource<Q
104104
QgsRectangle mFilterRect;
105105
QgsCoordinateTransform mTransform;
106106
QgsOgrDatasetSharedPtr mSharedDS = nullptr;
107+
QMutexLocker *mTransactionDSLocker = nullptr;
107108

108109
bool mFirstFieldIsFid = false;
109110
QgsFields mFieldsWithoutFid;

‎src/core/providers/ogr/qgsogrproviderutils.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,15 @@ class CORE_EXPORT QgsOgrProviderUtils
9595
#else
9696
QRecursiveMutex mutex;
9797
#endif
98+
QMutex sharedDSMutex;
9899
GDALDatasetH hDS = nullptr;
99100
QMap<QString, QgsOgrLayer *> setLayers;
100101
int refCount = 0;
101102
bool canBeShared = true;
102103

103104
DatasetWithLayers()
104105
#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)
105-
: mutex( QMutex::Recursive )
106+
: mutex( QMutex::Recursive ), sharedDSMutex( QMutex::Recursive )
106107
#endif
107108
{}
108109
};
@@ -304,6 +305,8 @@ class QgsOgrDataset
304305
QRecursiveMutex &mutex() { return mDs->mutex; }
305306
#endif
306307

308+
QMutex &sharedDSMutex() { return mDs->sharedDSMutex; }
309+
307310
bool executeSQLNoReturn( const QString &sql );
308311

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

0 commit comments

Comments
 (0)
Please sign in to comment.