Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #8375 from m-kuhn/deadlockInGeometryValidation
Backport of fix deadlock in geometry validation
  • Loading branch information
m-kuhn committed Oct 30, 2018
2 parents e1557c6 + 1f8e6a5 commit 06b4483
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 15 deletions.
Expand Up @@ -30,11 +30,12 @@ A feature pool is based on a vector layer and caches features.
QgsFeaturePool( QgsVectorLayer *layer );
virtual ~QgsFeaturePool();

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


Expand Down
10 changes: 5 additions & 5 deletions src/analysis/vector/geometry_checker/qgsfeaturepool.cpp
Expand Up @@ -36,7 +36,7 @@ QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer )

}

bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback )
{
QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
QgsFeature *cachedFeature = mFeatureCache.object( id );
Expand All @@ -47,11 +47,11 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
}
else
{
std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer );
std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer, feedback );

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

QgsFeatureIds QgsFeaturePool::getFeatures( const QgsFeatureRequest &request )
QgsFeatureIds QgsFeaturePool::getFeatures( const QgsFeatureRequest &request, QgsFeedback *feedback )
{
QgsFeatureIds fids;

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

QgsFeatureIterator it = source->getFeatures( request );
QgsFeature feature;
Expand Down
6 changes: 4 additions & 2 deletions src/analysis/vector/geometry_checker/qgsfeaturepool.h
Expand Up @@ -46,17 +46,19 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink SIP_ABSTRACT
* Retrieve the feature with the specified \a id into \a feature.
* It will be retrieved from the cache or from the underlying layer if unavailable.
* If the feature is neither available from the cache nor from the layer it will return false.
* If \a feedback is specified, the call may return if the feedback is canceled.
*/
bool getFeature( QgsFeatureId id, QgsFeature &feature );
bool getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback = nullptr );

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

/**
* Updates a feature in this pool.
Expand Down
Expand Up @@ -150,7 +150,7 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
if ( fid == currentFeature.id() )
continue;

if ( featurePool->getFeature( fid, compareFeature ) )
if ( featurePool->getFeature( fid, compareFeature, feedback ) )
{
if ( feedback->isCanceled() )
break;
Expand Down
66 changes: 65 additions & 1 deletion src/core/qgsthreadingutils.h
Expand Up @@ -20,7 +20,11 @@

#include "qgis_core.h"

#include "qgsfeedback.h"

#include <QThread>
#include <QSemaphore>
#include <memory>

/**
* \ingroup core
Expand All @@ -39,24 +43,84 @@ class CORE_EXPORT QgsThreadingUtils
* This is useful to quickly access information from objects that live on the
* main thread and copying this information into worker threads. Avoid running
* expensive code inside \a func.
* If a \a feedback is provided, it will observe if the feedback is canceled.
* In case the feedback is canceled before the main thread started to run the
* function, it will return without executing the function.
*
* \note Only works with Qt >= 5.10, earlier versions will execute the code
* in the worker thread.
*
* \since QGIS 3.4
*/
template <typename Func>
static void runOnMainThread( const Func &func )
static bool runOnMainThread( const Func &func, QgsFeedback *feedback = nullptr )
{
#if QT_VERSION >= QT_VERSION_CHECK( 5, 10, 0 )
// Make sure we only deal with the vector layer on the main thread where it lives.
// Anything else risks a crash.
if ( QThread::currentThread() == qApp->thread() )
{
func();
return true;
}
else
{
if ( feedback )
{
// This semaphore will block the worker thread until the main thread is ready.
// Ready means the event to execute the waitFunc has arrived in the event loop
// and is being executed.
QSemaphore semaphoreMainThreadReady( 1 );

// This semaphore will block the main thread until the worker thread is ready.
// Once the main thread is executing the waitFunc, it will wait for this semaphore
// to be released. This way we can make sure that
QSemaphore semaphoreWorkerThreadReady( 1 );

// Acquire both semaphores. We want the main thread and the current thread to be blocked
// until it's save to continue.
semaphoreMainThreadReady.acquire();
semaphoreWorkerThreadReady.acquire();

std::function<void()> waitFunc = [&semaphoreMainThreadReady, &semaphoreWorkerThreadReady]()
{
// This function is executed on the main thread. As soon as it's executed
// it will tell the worker thread that the main thread is blocked by releasing
// the semaphore.
semaphoreMainThreadReady.release();

// ... and wait for the worker thread to release its semaphore
semaphoreWorkerThreadReady.acquire();
};

QMetaObject::invokeMethod( qApp, waitFunc, Qt::QueuedConnection );

// while we are in the event queue for the main thread and not yet
// being executed, check all 100 ms if the feedback is canceled.
while ( !semaphoreMainThreadReady.tryAcquire( 1, 100 ) )
{
if ( feedback->isCanceled() )
{
semaphoreWorkerThreadReady.release();
return false;
}
}

// finally, the main thread is blocked and we are (most likely) not canceled.
// let's do the real work!!
func();

// work done -> tell the main thread he may continue
semaphoreWorkerThreadReady.release();
return true;
}
QMetaObject::invokeMethod( qApp, func, Qt::BlockingQueuedConnection );
return true;
}
#else
Q_UNUSED( feedback )
func();
return true;
#endif
}

Expand Down
10 changes: 6 additions & 4 deletions src/core/qgsvectorlayerutils.cpp
Expand Up @@ -510,14 +510,16 @@ QgsFeature QgsVectorLayerUtils::duplicateFeature( QgsVectorLayer *layer, const Q
return newFeature;
}

std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSource( QPointer<QgsVectorLayer> layer )
std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSource( QPointer<QgsVectorLayer> layer, QgsFeedback *feedback )
{
std::unique_ptr<QgsVectorLayerFeatureSource> featureSource;

auto getFeatureSource = [ layer, &featureSource ]
auto getFeatureSource = [ layer, &featureSource, feedback ]
{
#if QT_VERSION >= QT_VERSION_CHECK( 5, 10, 0 )
Q_ASSERT( QThread::currentThread() == qApp->thread() );
Q_ASSERT( QThread::currentThread() == qApp->thread() || feedback );
#else
Q_UNUSED( feedback )
#endif
QgsVectorLayer *lyr = layer.data();

Expand All @@ -527,7 +529,7 @@ std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSour
}
};

QgsThreadingUtils::runOnMainThread( getFeatureSource );
QgsThreadingUtils::runOnMainThread( getFeatureSource, feedback );

return featureSource;
}
Expand Down
5 changes: 4 additions & 1 deletion src/core/qgsvectorlayerutils.h
Expand Up @@ -162,11 +162,14 @@ class CORE_EXPORT QgsVectorLayerUtils
* This should be used in scenarios, where a ``QWeakPointer<QgsVectorLayer>`` is kept in a thread
* and features should be fetched from this layer. Using the layer directly is not safe to do.
* The result will be ``nullptr`` if the layer has been deleted.
* If \a feedback is specified, the call will return if the feedback is canceled.
* Returns a new feature source for the \a layer. The source may be a nullptr if the layer no longer
* exists or if the feedback is canceled.
*
* \note Requires Qt >= 5.10 to make use of the thread-safe implementation
* \since QGIS 3.4
*/
static std::unique_ptr<QgsVectorLayerFeatureSource> getFeatureSource( QPointer<QgsVectorLayer> layer ) SIP_SKIP;
static std::unique_ptr<QgsVectorLayerFeatureSource> getFeatureSource( QPointer<QgsVectorLayer> layer, QgsFeedback *feedback = nullptr ) SIP_SKIP;

/**
* Matches the attributes in \a feature to the specified \a fields.
Expand Down

0 comments on commit 06b4483

Please sign in to comment.