Skip to content

Commit 02ec785

Browse files
m-kuhnm-kuhn
committedOct 29, 2018
Fix deadlock in geometry validation
Fix a deadlock in geometry validation. This happens when the mainthread "waits for finished" of a checker thread. What happened in this case was, that the main thread canceled the feedback and waited for any jobs to finish. If a job was waiting for a feature source (or something else to be executed on the main thread) this resulted in a mighty deadlock. What we do here is that we regularly check if we ought to cancel while waiting for our slot on the main thread and can bail out if we should cancel before the main thread gets around to take care of our function. The difference is, we still execute the code on the background thread and make sure that the main thread is not doing anything during this time to avoid working on the same data structures in parallel.
·
ltr-3_40final-3_6_0
1 parent b00b3f6 commit 02ec785

File tree

7 files changed

+81
-15
lines changed

7 files changed

+81
-15
lines changed
 

‎python/analysis/auto_generated/vector/geometry_checker/qgsfeaturepool.sip.in

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ A feature pool is based on a vector layer and caches features.
3030
QgsFeaturePool( QgsVectorLayer *layer );
3131
virtual ~QgsFeaturePool();
3232

33-
bool getFeature( QgsFeatureId id, QgsFeature &feature );
33+
bool getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback = 0 );
3434
%Docstring
3535
Retrieve the feature with the specified ``id`` into ``feature``.
3636
It will be retrieved from the cache or from the underlying layer if unavailable.
3737
If the feature is neither available from the cache nor from the layer it will return false.
38+
If ``feedback`` is specified, the call may return if the feedback is canceled.
3839
%End
3940

4041

‎src/analysis/vector/geometry_checker/qgsfeaturepool.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer )
3636

3737
}
3838

39-
bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
39+
bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback )
4040
{
4141
QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
4242
QgsFeature *cachedFeature = mFeatureCache.object( id );
@@ -47,11 +47,11 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
4747
}
4848
else
4949
{
50-
std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer );
50+
std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer, feedback );
5151

5252
// Feature not in cache, retrieve from layer
5353
// TODO: avoid always querying all attributes (attribute values are needed when merging by attribute)
54-
if ( !source->getFeatures( QgsFeatureRequest( id ) ).nextFeature( feature ) )
54+
if ( !source || !source->getFeatures( QgsFeatureRequest( id ) ).nextFeature( feature ) )
5555
{
5656
return false;
5757
}
@@ -62,11 +62,11 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
6262
return true;
6363
}
6464

65-
QgsFeatureIds QgsFeaturePool::getFeatures( const QgsFeatureRequest &request )
65+
QgsFeatureIds QgsFeaturePool::getFeatures( const QgsFeatureRequest &request, QgsFeedback *feedback )
6666
{
6767
QgsFeatureIds fids;
6868

69-
std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer );
69+
std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer, feedback );
7070

7171
QgsFeatureIterator it = source->getFeatures( request );
7272
QgsFeature feature;

‎src/analysis/vector/geometry_checker/qgsfeaturepool.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,19 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink SIP_ABSTRACT
4646
* Retrieve the feature with the specified \a id into \a feature.
4747
* It will be retrieved from the cache or from the underlying layer if unavailable.
4848
* If the feature is neither available from the cache nor from the layer it will return false.
49+
* If \a feedback is specified, the call may return if the feedback is canceled.
4950
*/
50-
bool getFeature( QgsFeatureId id, QgsFeature &feature );
51+
bool getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback = nullptr );
5152

5253
/**
5354
* Get features for the provided \a request. No features will be fetched
5455
* from the cache and the request is sent directly to the underlying feature source.
5556
* Results of the request are cached in the pool and the ids of all the features
5657
* are returned. This can be used to warm the cache for a particular area of interest
5758
* (bounding box) or other set of features.
59+
* If \a feedback is specified, the call may return if the feedback is canceled.
5860
*/
59-
QgsFeatureIds getFeatures( const QgsFeatureRequest &request ) SIP_SKIP;
61+
QgsFeatureIds getFeatures( const QgsFeatureRequest &request, QgsFeedback *feedback = nullptr ) SIP_SKIP;
6062

6163
/**
6264
* Updates a feature in this pool.

‎src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
150150
if ( fid == currentFeature.id() )
151151
continue;
152152

153-
if ( featurePool->getFeature( fid, compareFeature ) )
153+
if ( featurePool->getFeature( fid, compareFeature, feedback ) )
154154
{
155155
if ( feedback->isCanceled() )
156156
break;

‎src/core/qgsthreadingutils.h

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@
2020

2121
#include "qgis_core.h"
2222

23+
#include "qgsfeedback.h"
24+
2325
#include <QThread>
26+
#include <QSemaphore>
27+
#include <memory>
2428

2529
/**
2630
* \ingroup core
@@ -39,24 +43,83 @@ class CORE_EXPORT QgsThreadingUtils
3943
* This is useful to quickly access information from objects that live on the
4044
* main thread and copying this information into worker threads. Avoid running
4145
* expensive code inside \a func.
46+
* If a \a feedback is provided, it will observe if the feedback is canceled.
47+
* In case the feedback is canceled before the main thread started to run the
48+
* function, it will return without executing the function.
4249
*
4350
* \note Only works with Qt >= 5.10, earlier versions will execute the code
4451
* in the worker thread.
4552
*
4653
* \since QGIS 3.4
4754
*/
4855
template <typename Func>
49-
static void runOnMainThread( const Func &func )
56+
static bool runOnMainThread( const Func &func, QgsFeedback *feedback = nullptr )
5057
{
5158
#if QT_VERSION >= QT_VERSION_CHECK( 5, 10, 0 )
5259
// Make sure we only deal with the vector layer on the main thread where it lives.
5360
// Anything else risks a crash.
5461
if ( QThread::currentThread() == qApp->thread() )
62+
{
5563
func();
64+
return true;
65+
}
5666
else
67+
{
68+
if ( feedback )
69+
{
70+
// This semaphore will block the worker thread until the main thread is ready.
71+
// Ready means the event to execute the waitFunc has arrived in the event loop
72+
// and is being executed.
73+
QSemaphore semaphoreMainThreadReady( 1 );
74+
75+
// This semaphore will block the main thread until the worker thread is ready.
76+
// Once the main thread is executing the waitFunc, it will wait for this semaphore
77+
// to be released. This way we can make sure that
78+
QSemaphore semaphoreWorkerThreadReady( 1 );
79+
80+
// Acquire both semaphores. We want the main thread and the current thread to be blocked
81+
// until it's save to continue.
82+
semaphoreMainThreadReady.acquire();
83+
semaphoreWorkerThreadReady.acquire();
84+
85+
std::function<void()> waitFunc = [&semaphoreMainThreadReady, &semaphoreWorkerThreadReady]()
86+
{
87+
// This function is executed on the main thread. As soon as it's executed
88+
// it will tell the worker thread that the main thread is blocked by releasing
89+
// the semaphore.
90+
semaphoreMainThreadReady.release();
91+
92+
// ... and wait for the worker thread to release its semaphore
93+
semaphoreWorkerThreadReady.acquire();
94+
};
95+
96+
QMetaObject::invokeMethod( qApp, waitFunc, Qt::QueuedConnection );
97+
98+
// while we are in the event queue for the main thread and not yet
99+
// being executed, check all 100 ms if the feedback is canceled.
100+
while ( !semaphoreMainThreadReady.tryAcquire( 1, 100 ) )
101+
{
102+
if ( feedback->isCanceled() )
103+
{
104+
semaphoreWorkerThreadReady.release();
105+
return false;
106+
}
107+
}
108+
109+
// finally, the main thread is blocked and we are (most likely) not canceled.
110+
// let's do the real work!!
111+
func();
112+
113+
// work done -> tell the main thread he may continue
114+
semaphoreWorkerThreadReady.release();
115+
return true;
116+
}
57117
QMetaObject::invokeMethod( qApp, func, Qt::BlockingQueuedConnection );
118+
return true;
119+
}
58120
#else
59121
func();
122+
return true;
60123
#endif
61124
}
62125

‎src/core/qgsvectorlayerutils.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -510,14 +510,14 @@ QgsFeature QgsVectorLayerUtils::duplicateFeature( QgsVectorLayer *layer, const Q
510510
return newFeature;
511511
}
512512

513-
std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSource( QPointer<QgsVectorLayer> layer )
513+
std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSource( QPointer<QgsVectorLayer> layer, QgsFeedback *feedback )
514514
{
515515
std::unique_ptr<QgsVectorLayerFeatureSource> featureSource;
516516

517-
auto getFeatureSource = [ layer, &featureSource ]
517+
auto getFeatureSource = [ layer, &featureSource, feedback ]
518518
{
519519
#if QT_VERSION >= QT_VERSION_CHECK( 5, 10, 0 )
520-
Q_ASSERT( QThread::currentThread() == qApp->thread() );
520+
Q_ASSERT( QThread::currentThread() == qApp->thread() || feedback );
521521
#endif
522522
QgsVectorLayer *lyr = layer.data();
523523

@@ -527,7 +527,7 @@ std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSour
527527
}
528528
};
529529

530-
QgsThreadingUtils::runOnMainThread( getFeatureSource );
530+
QgsThreadingUtils::runOnMainThread( getFeatureSource, feedback );
531531

532532
return featureSource;
533533
}

‎src/core/qgsvectorlayerutils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ class CORE_EXPORT QgsVectorLayerUtils
166166
* \note Requires Qt >= 5.10 to make use of the thread-safe implementation
167167
* \since QGIS 3.4
168168
*/
169-
static std::unique_ptr<QgsVectorLayerFeatureSource> getFeatureSource( QPointer<QgsVectorLayer> layer ) SIP_SKIP;
169+
static std::unique_ptr<QgsVectorLayerFeatureSource> getFeatureSource( QPointer<QgsVectorLayer> layer, QgsFeedback *feedback ) SIP_SKIP;
170170

171171
/**
172172
* Matches the attributes in \a feature to the specified \a fields.

0 commit comments

Comments
 (0)
Please sign in to comment.