From d2f52bf400c00125771c87826428aeb6eec8c3f5 Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Tue, 4 Sep 2018 15:11:28 +0200 Subject: [PATCH 1/6] Add threadsafe method to get featuresource from layer --- src/core/qgsvectorlayerutils.cpp | 28 ++++++++++++++++++++++++++++ src/core/qgsvectorlayerutils.h | 12 ++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/core/qgsvectorlayerutils.cpp b/src/core/qgsvectorlayerutils.cpp index f78813f8857..1dcd78342d6 100644 --- a/src/core/qgsvectorlayerutils.cpp +++ b/src/core/qgsvectorlayerutils.cpp @@ -498,6 +498,34 @@ QgsFeature QgsVectorLayerUtils::duplicateFeature( QgsVectorLayer *layer, const Q return newFeature; } +std::unique_ptr QgsVectorLayerUtils::getFeatureSource( QWeakPointer layer ) +{ + std::unique_ptr featureSource; + + auto getFeatureSource = [ layer, &featureSource ] + { + QgsVectorLayer *lyr = layer.data(); + + if ( lyr ) + { + featureSource.reset( new QgsVectorLayerFeatureSource( lyr ) ); + } + }; + +#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() ) + getFeatureSource(); + else + QMetaObject::invokeMethod( qApp, getFeatureSource, Qt::BlockingQueuedConnection ); +#else + getFeatureSource(); +#endif + + return featureSource; +} + QList QgsVectorLayerUtils::QgsDuplicateFeatureContext::layers() const { QList layers; diff --git a/src/core/qgsvectorlayerutils.h b/src/core/qgsvectorlayerutils.h index 68d35803e1d..1f3a043a493 100644 --- a/src/core/qgsvectorlayerutils.h +++ b/src/core/qgsvectorlayerutils.h @@ -19,6 +19,7 @@ #include "qgis_core.h" #include "qgsvectorlayer.h" #include "qgsgeometry.h" +#include "qgsvectorlayerfeatureiterator.h" /** * \ingroup core @@ -155,6 +156,17 @@ class CORE_EXPORT QgsVectorLayerUtils */ static QgsFeature duplicateFeature( QgsVectorLayer *layer, const QgsFeature &feature, QgsProject *project, int depth, QgsDuplicateFeatureContext &duplicateFeatureContext SIP_OUT ); + /** + * Gets the feature source from a weak QgsVectorLayer pointer. + * This method is thread-safe but will block the main thread for execution. Executing it from the main + * thread is safe too. + * This should be used in scenarios, where a ``QWeakPointer`` is kept in a thread + * and features should be fetched from this layer. Using the layer directly is not safe to do. + * + * \note Requires Qt >= 5.10 to make use of the thread-safe implementation + * \since QGIS 3.4 + */ + static std::unique_ptr getFeatureSource( QWeakPointer layer ) SIP_SKIP; }; From 44ce8977d9fd2361d08e6be4829126882123804d Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Tue, 4 Sep 2018 22:04:51 +0200 Subject: [PATCH 2/6] Use QPointer instead of QWeakPointer --- src/core/qgsvectorlayerutils.cpp | 5 ++++- src/core/qgsvectorlayerutils.h | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/core/qgsvectorlayerutils.cpp b/src/core/qgsvectorlayerutils.cpp index 1dcd78342d6..4bf76496cdd 100644 --- a/src/core/qgsvectorlayerutils.cpp +++ b/src/core/qgsvectorlayerutils.cpp @@ -498,12 +498,15 @@ QgsFeature QgsVectorLayerUtils::duplicateFeature( QgsVectorLayer *layer, const Q return newFeature; } -std::unique_ptr QgsVectorLayerUtils::getFeatureSource( QWeakPointer layer ) +std::unique_ptr QgsVectorLayerUtils::getFeatureSource( QPointer layer ) { std::unique_ptr featureSource; auto getFeatureSource = [ layer, &featureSource ] { +#if QT_VERSION >= QT_VERSION_CHECK( 5, 10, 0 ) + Q_ASSERT( QThread::currentThread() == qApp->thread() ); +#endif QgsVectorLayer *lyr = layer.data(); if ( lyr ) diff --git a/src/core/qgsvectorlayerutils.h b/src/core/qgsvectorlayerutils.h index 1f3a043a493..6e617d5019f 100644 --- a/src/core/qgsvectorlayerutils.h +++ b/src/core/qgsvectorlayerutils.h @@ -157,7 +157,7 @@ class CORE_EXPORT QgsVectorLayerUtils static QgsFeature duplicateFeature( QgsVectorLayer *layer, const QgsFeature &feature, QgsProject *project, int depth, QgsDuplicateFeatureContext &duplicateFeatureContext SIP_OUT ); /** - * Gets the feature source from a weak QgsVectorLayer pointer. + * Gets the feature source from a QgsVectorLayer pointer. * This method is thread-safe but will block the main thread for execution. Executing it from the main * thread is safe too. * This should be used in scenarios, where a ``QWeakPointer`` is kept in a thread @@ -166,7 +166,7 @@ class CORE_EXPORT QgsVectorLayerUtils * \note Requires Qt >= 5.10 to make use of the thread-safe implementation * \since QGIS 3.4 */ - static std::unique_ptr getFeatureSource( QWeakPointer layer ) SIP_SKIP; + static std::unique_ptr getFeatureSource( QPointer layer ) SIP_SKIP; }; From 75bb5c40a7691c98770fe0cf4fe6ecc7fdc92c71 Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Tue, 4 Sep 2018 22:05:43 +0200 Subject: [PATCH 3/6] Add test for QgsVectorLayerUtils::getFeatureSource --- tests/src/core/CMakeLists.txt | 1 + tests/src/core/testqgsvectorlayerutils.cpp | 120 +++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 tests/src/core/testqgsvectorlayerutils.cpp diff --git a/tests/src/core/CMakeLists.txt b/tests/src/core/CMakeLists.txt index 618d0d814cf..83869219db8 100755 --- a/tests/src/core/CMakeLists.txt +++ b/tests/src/core/CMakeLists.txt @@ -192,6 +192,7 @@ SET(TESTS testqgsvectorlayercache.cpp testqgsvectorlayerjoinbuffer.cpp testqgsvectorlayer.cpp + testqgsvectorlayerutils.cpp testziplayer.cpp testqgsmeshlayer.cpp testqgsmeshlayerrenderer.cpp diff --git a/tests/src/core/testqgsvectorlayerutils.cpp b/tests/src/core/testqgsvectorlayerutils.cpp new file mode 100644 index 00000000000..0ffd38f3aa4 --- /dev/null +++ b/tests/src/core/testqgsvectorlayerutils.cpp @@ -0,0 +1,120 @@ +/*************************************************************************** + test_template.cpp + -------------------------------------- + Date : Sun Sep 16 12:22:23 AKDT 2007 + Copyright : (C) 2007 by Gary E. Sherman + Email : sherman at mrcc dot com + *************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ +#include "qgstest.h" + +#include "qgsvectorlayerutils.h" + +/** + * \ingroup UnitTests + * This is a unit test for the vector layer class. + */ +class TestQgsVectorLayerUtils : public QObject +{ + Q_OBJECT + public: + TestQgsVectorLayerUtils() = default; + + private: + bool mTestHasError = false ; + QgsMapLayer *mpPointsLayer = nullptr; + QgsMapLayer *mpLinesLayer = nullptr; + QgsMapLayer *mpPolysLayer = nullptr; + QgsVectorLayer *mpNonSpatialLayer = nullptr; + QString mTestDataDir; + QString mReport; + + private slots: + + void initTestCase(); // will be called before the first testfunction is executed. + void cleanupTestCase(); // will be called after the last testfunction was executed. + void init() {} // will be called before each testfunction is executed. + void cleanup() {} // will be called after every testfunction. + + void testGetFeatureSource(); +}; + +void TestQgsVectorLayerUtils::initTestCase() +{ + QgsApplication::init(); + QgsApplication::initQgis(); + QgsApplication::showSettings(); + +} + +void TestQgsVectorLayerUtils::cleanupTestCase() +{ + QgsApplication::exitQgis(); +} + +class FeatureFetcher : public QThread +{ + Q_OBJECT + + public: + FeatureFetcher( QPointer layer ) + : mLayer( layer ) + { + } + + void run() override + { + QgsFeature feat; + QgsVectorLayerUtils::getFeatureSource( mLayer ).get()->getFeatures().nextFeature( feat ); + emit resultReady( feat.attribute( QStringLiteral( "col1" ) ) ); + } + + signals: + void resultReady( const QVariant &attribute ); + + private: + QPoinst mLayer; +}; + + +void TestQgsVectorLayerUtils::testGetFeatureSource() +{ + QgsVectorLayer *vl = new QgsVectorLayer( QStringLiteral( "Point?field=col1:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ); + vl->startEditing(); + QgsFeature f1( vl->fields(), 1 ); + f1.setAttribute( QStringLiteral( "col1" ), 10 ); + vl->addFeature( f1 ); + + QPointer vlPtr( vl ); + + QgsFeature feat; + QgsVectorLayerUtils::getFeatureSource( vlPtr ).get()->getFeatures().nextFeature( feat ); + QCOMPARE( feat.attribute( QStringLiteral( "col1" ) ).toInt(), 10 ); + + FeatureFetcher *thread = new FeatureFetcher(); + + bool finished = false; + QVariant result; + connect( thread, &FeatureFetcher::resultReady, this, [finished, result]( const QVariant & res ) + { + finished = true; + result = res; + } ); + connect( thread, &QThread::finished, thread, &QThread::deleteLater ); + + thread->start(); + while ( !finished ) + QCoreApplication::processEvents(); + QCOMPARE( result.toInt(), 10 ); + + thread->quit(); +} + +QGSTEST_MAIN( TestQgsVectorLayerUtils ) +#include "testqgsvectorlayerutils.moc" From e23167ee3ddf1380465895283494f7b0c151b081 Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Wed, 5 Sep 2018 05:32:37 +0200 Subject: [PATCH 4/6] Fix some test issues --- tests/src/core/testqgsvectorlayerutils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/core/testqgsvectorlayerutils.cpp b/tests/src/core/testqgsvectorlayerutils.cpp index 0ffd38f3aa4..d258c32a428 100644 --- a/tests/src/core/testqgsvectorlayerutils.cpp +++ b/tests/src/core/testqgsvectorlayerutils.cpp @@ -79,7 +79,7 @@ class FeatureFetcher : public QThread void resultReady( const QVariant &attribute ); private: - QPoinst mLayer; + QPointer mLayer; }; @@ -97,7 +97,7 @@ void TestQgsVectorLayerUtils::testGetFeatureSource() QgsVectorLayerUtils::getFeatureSource( vlPtr ).get()->getFeatures().nextFeature( feat ); QCOMPARE( feat.attribute( QStringLiteral( "col1" ) ).toInt(), 10 ); - FeatureFetcher *thread = new FeatureFetcher(); + FeatureFetcher *thread = new FeatureFetcher( vlPtr ); bool finished = false; QVariant result; From 26626ea3d8a9b823dabbab67c1a15d9642552baa Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Wed, 5 Sep 2018 06:40:26 +0200 Subject: [PATCH 5/6] Fix test --- tests/src/core/testqgsvectorlayerutils.cpp | 41 +++++++++++++--------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/tests/src/core/testqgsvectorlayerutils.cpp b/tests/src/core/testqgsvectorlayerutils.cpp index d258c32a428..1c81f919281 100644 --- a/tests/src/core/testqgsvectorlayerutils.cpp +++ b/tests/src/core/testqgsvectorlayerutils.cpp @@ -26,15 +26,6 @@ class TestQgsVectorLayerUtils : public QObject public: TestQgsVectorLayerUtils() = default; - private: - bool mTestHasError = false ; - QgsMapLayer *mpPointsLayer = nullptr; - QgsMapLayer *mpLinesLayer = nullptr; - QgsMapLayer *mpPolysLayer = nullptr; - QgsVectorLayer *mpNonSpatialLayer = nullptr; - QString mTestDataDir; - QString mReport; - private slots: void initTestCase(); // will be called before the first testfunction is executed. @@ -71,7 +62,9 @@ class FeatureFetcher : public QThread void run() override { QgsFeature feat; - QgsVectorLayerUtils::getFeatureSource( mLayer ).get()->getFeatures().nextFeature( feat ); + auto fs = QgsVectorLayerUtils::getFeatureSource( mLayer ); + if ( fs ) + fs->getFeatures().nextFeature( feat ); emit resultReady( feat.attribute( QStringLiteral( "col1" ) ) ); } @@ -85,35 +78,51 @@ class FeatureFetcher : public QThread void TestQgsVectorLayerUtils::testGetFeatureSource() { - QgsVectorLayer *vl = new QgsVectorLayer( QStringLiteral( "Point?field=col1:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ); + std::unique_ptr vl = qgis::make_unique( QStringLiteral( "Point?field=col1:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ); vl->startEditing(); QgsFeature f1( vl->fields(), 1 ); f1.setAttribute( QStringLiteral( "col1" ), 10 ); vl->addFeature( f1 ); - QPointer vlPtr( vl ); + QPointer vlPtr( vl.get() ); QgsFeature feat; - QgsVectorLayerUtils::getFeatureSource( vlPtr ).get()->getFeatures().nextFeature( feat ); + QgsVectorLayerUtils::getFeatureSource( vlPtr )->getFeatures().nextFeature( feat ); QCOMPARE( feat.attribute( QStringLiteral( "col1" ) ).toInt(), 10 ); FeatureFetcher *thread = new FeatureFetcher( vlPtr ); bool finished = false; QVariant result; - connect( thread, &FeatureFetcher::resultReady, this, [finished, result]( const QVariant & res ) + + auto onResultReady = [&finished, &result]( const QVariant & res ) { finished = true; result = res; - } ); + }; + + connect( thread, &FeatureFetcher::resultReady, this, onResultReady ); connect( thread, &QThread::finished, thread, &QThread::deleteLater ); thread->start(); while ( !finished ) QCoreApplication::processEvents(); QCOMPARE( result.toInt(), 10 ); - thread->quit(); + + FeatureFetcher *thread2 = new FeatureFetcher( vlPtr ); + + finished = false; + result = QVariant(); + connect( thread2, &FeatureFetcher::resultReady, this, onResultReady ); + connect( thread2, &QThread::finished, thread, &QThread::deleteLater ); + + vl.reset(); + thread2->start(); + while ( !finished ) + QCoreApplication::processEvents(); + QVERIFY( result.isNull() ); + thread2->quit(); } QGSTEST_MAIN( TestQgsVectorLayerUtils ) From 86f429375d76b41967a4f5b0e765e16513b50ab4 Mon Sep 17 00:00:00 2001 From: Matthias Kuhn Date: Wed, 5 Sep 2018 06:42:24 +0200 Subject: [PATCH 6/6] Doxygen --- src/core/qgsvectorlayerutils.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/qgsvectorlayerutils.h b/src/core/qgsvectorlayerutils.h index 6e617d5019f..68876377246 100644 --- a/src/core/qgsvectorlayerutils.h +++ b/src/core/qgsvectorlayerutils.h @@ -162,6 +162,7 @@ class CORE_EXPORT QgsVectorLayerUtils * thread is safe too. * This should be used in scenarios, where a ``QWeakPointer`` 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. * * \note Requires Qt >= 5.10 to make use of the thread-safe implementation * \since QGIS 3.4