Skip to content
Permalink
Browse files

Make QgsLocator more thread safe

- add a clone() method to filters, and always search using the
clone instead of the original filter
- add a prepare() method to filters, which is always run in the
main thread and can be used to prepare the filter for safe
background execution (e.g. creating feature iterators in advance)
- don't use QtConcurrent to perform searches in background threads,
since it is not safe to use with QObjects
- instead manually create threads and ensure that cloned objects
are always moved to the thread that they will run in, to ensure
that they correctly have thread affinity with the thread in which
they are executed
  • Loading branch information
nyalldawson committed Feb 12, 2018
1 parent 5431a2d commit 981afb3da1a29a8643bfa9744875e5f9149f416c
@@ -30,9 +30,6 @@ PyQgsSpatialiteProvider
# Flaky, see https://travis-ci.org/qgis/QGIS/jobs/297708174
PyQgsServerAccessControl

# Flaky, see https://dash.orfeo-toolbox.org/testDetails.php?test=61670866&build=297397
PyQgsLocator

# Need a local postgres installation
PyQgsAuthManagerPKIPostgresTest
PyQgsAuthManagerPasswordPostgresTest
@@ -72,6 +72,12 @@ Abstract base class for filters which collect locator results.
QgsLocatorFilter( QObject *parent = 0 );
%Docstring
Constructor for QgsLocatorFilter.
%End

virtual QgsLocatorFilter *clone() const = 0 /Factory/;
%Docstring
Creates a clone of the filter. New requests are always executed in a
clone of the original filter.
%End

virtual QString name() const = 0;
@@ -108,6 +114,20 @@ results from this filter.
be ignored.
%End

virtual void prepare( const QString &string, const QgsLocatorContext &context );
%Docstring
Prepares the filter instance for an upcoming search for the specified ``string``. This method is always called
from the main thread, and individual filter subclasses should perform whatever
tasks are required in order to allow a subsequent search to safely execute
on a background thread.
%End

void executeSearchAndDelete( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback );
%Docstring
Executes a search for this filter instance, and then deletes the current instance
of the filter.
%End

virtual void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) = 0;
%Docstring
Retrieves the filter results for a specified search ``string``. The ``context``
@@ -190,6 +210,8 @@ custom configuration widget.

signals:

void finished();

void resultFetched( const QgsLocatorResult &result );
%Docstring
Should be emitted by filters whenever they encounter a matching result
@@ -39,6 +39,10 @@ class AlgorithmLocatorFilter(QgsLocatorFilter):

def __init__(self, parent=None):
super(AlgorithmLocatorFilter, self).__init__(parent)
self.found_results = []

def clone(self):
return AlgorithmLocatorFilter()

def name(self):
return 'processing_alg'
@@ -52,10 +56,10 @@ def priority(self):
def prefix(self):
return 'a'

def fetchResults(self, string, context, feedback):
def prepare(self, string, context):
# collect results in main thread, since this method is inexpensive and
# accessing the processing registry is not thread safe
for a in QgsApplication.processingRegistry().algorithms():
if feedback.isCanceled():
return
if a.flags() & QgsProcessingAlgorithm.FlagHideFromToolbox:
continue

@@ -69,7 +73,11 @@ def fetchResults(self, string, context, feedback):
result.score = float(len(string)) / len(a.displayName())
else:
result.score = 0
self.resultFetched.emit(result)
self.found_results.append(result)

def fetchResults(self, string, context, feedback):
for result in self.found_results:
self.resultFetched.emit(result)

def triggerResult(self, result):
alg = QgsApplication.processingRegistry().createAlgorithmById(result.userData)
@@ -31,28 +31,39 @@ QgsLayerTreeLocatorFilter::QgsLayerTreeLocatorFilter( QObject *parent )
: QgsLocatorFilter( parent )
{}

void QgsLayerTreeLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
QgsLayerTreeLocatorFilter *QgsLayerTreeLocatorFilter::clone() const
{
return new QgsLayerTreeLocatorFilter();
}

void QgsLayerTreeLocatorFilter::prepare( const QString &string, const QgsLocatorContext & )
{
// collect results in main thread, since this method is inexpensive and
// accessing the layer tree root is not thread safe
QgsLayerTree *tree = QgsProject::instance()->layerTreeRoot();
QList<QgsLayerTreeLayer *> layers = tree->findLayers();
Q_FOREACH ( QgsLayerTreeLayer *layer, layers )
const QList<QgsLayerTreeLayer *> layers = tree->findLayers();
for ( QgsLayerTreeLayer *layer : layers )
{
if ( feedback->isCanceled() )
return;

if ( layer->layer() && stringMatches( layer->layer()->name(), string ) )
{
QgsLocatorResult result;
result.filter = this;
result.displayString = layer->layer()->name();
result.userData = layer->layerId();
result.icon = QgsMapLayerModel::iconForLayer( layer->layer() );
result.score = static_cast< double >( string.length() ) / layer->layer()->name().length();
emit resultFetched( result );
mResults.append( result );
}
}
}

void QgsLayerTreeLocatorFilter::fetchResults( const QString &, const QgsLocatorContext &, QgsFeedback * )
{
for ( const QgsLocatorResult &result : qgis::as_const( mResults ) )
{
emit resultFetched( result );
}
}

void QgsLayerTreeLocatorFilter::triggerResult( const QgsLocatorResult &result )
{
QString layerId = result.userData.toString();
@@ -68,27 +79,39 @@ QgsLayoutLocatorFilter::QgsLayoutLocatorFilter( QObject *parent )
: QgsLocatorFilter( parent )
{}

void QgsLayoutLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
QgsLayoutLocatorFilter *QgsLayoutLocatorFilter::clone() const
{
return new QgsLayoutLocatorFilter();
}

void QgsLayoutLocatorFilter::prepare( const QString &string, const QgsLocatorContext & )
{
// collect results in main thread, since this method is inexpensive and
// accessing the project layouts is not thread safe

const QList< QgsMasterLayoutInterface * > layouts = QgsProject::instance()->layoutManager()->layouts();
for ( QgsMasterLayoutInterface *layout : layouts )
{
if ( feedback->isCanceled() )
return;

if ( layout && stringMatches( layout->name(), string ) )
{
QgsLocatorResult result;
result.filter = this;
result.displayString = layout->name();
result.userData = layout->name();
//result.icon = QgsMapLayerModel::iconForLayer( layer->layer() );
result.score = static_cast< double >( string.length() ) / layout->name().length();
emit resultFetched( result );
mResults.append( result );
}
}
}

void QgsLayoutLocatorFilter::fetchResults( const QString &, const QgsLocatorContext &, QgsFeedback * )
{
for ( const QgsLocatorResult &result : qgis::as_const( mResults ) )
{
emit resultFetched( result );
}
}

void QgsLayoutLocatorFilter::triggerResult( const QgsLocatorResult &result )
{
QString layoutName = result.userData.toString();
@@ -109,19 +132,32 @@ QgsActionLocatorFilter::QgsActionLocatorFilter( const QList<QWidget *> &parentOb
setUseWithoutPrefix( false );
}

void QgsActionLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
QgsActionLocatorFilter *QgsActionLocatorFilter::clone() const
{
return new QgsActionLocatorFilter( mActionParents );
}

void QgsActionLocatorFilter::prepare( const QString &string, const QgsLocatorContext & )
{
// collect results in main thread, since this method is inexpensive and
// accessing the gui actions is not thread safe

QList<QAction *> found;

Q_FOREACH ( QWidget *object, mActionParents )
for ( QWidget *object : qgis::as_const( mActionParents ) )
{
if ( feedback->isCanceled() )
return;

searchActions( string, object, found );
}
}

void QgsActionLocatorFilter::fetchResults( const QString &, const QgsLocatorContext &, QgsFeedback * )
{
for ( const QgsLocatorResult &result : qgis::as_const( mResults ) )
{
emit resultFetched( result );
}
}

void QgsActionLocatorFilter::triggerResult( const QgsLocatorResult &result )
{
QAction *action = qobject_cast< QAction * >( qvariant_cast<QObject *>( result.userData ) );
@@ -131,8 +167,8 @@ void QgsActionLocatorFilter::triggerResult( const QgsLocatorResult &result )

void QgsActionLocatorFilter::searchActions( const QString &string, QWidget *parent, QList<QAction *> &found )
{
QList< QWidget *> children = parent->findChildren<QWidget *>();
Q_FOREACH ( QWidget *widget, children )
const QList< QWidget *> children = parent->findChildren<QWidget *>();
for ( QWidget *widget : children )
{
searchActions( string, widget, found );
}
@@ -154,12 +190,11 @@ void QgsActionLocatorFilter::searchActions( const QString &string, QWidget *pare
if ( stringMatches( searchText, string ) )
{
QgsLocatorResult result;
result.filter = this;
result.displayString = searchText;
result.userData = QVariant::fromValue( action );
result.icon = action->icon();
result.score = static_cast< double >( string.length() ) / searchText.length();
emit resultFetched( result );
mResults.append( result );
found << action;
}
}
@@ -171,7 +206,12 @@ QgsActiveLayerFeaturesLocatorFilter::QgsActiveLayerFeaturesLocatorFilter( QObjec
setUseWithoutPrefix( false );
}

void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
QgsActiveLayerFeaturesLocatorFilter *QgsActiveLayerFeaturesLocatorFilter::clone() const
{
return new QgsActiveLayerFeaturesLocatorFilter();
}

void QgsActiveLayerFeaturesLocatorFilter::prepare( const QString &string, const QgsLocatorContext & )
{
if ( string.length() < 3 )
return;
@@ -183,11 +223,9 @@ void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, c
if ( !layer )
return;

int found = 0;
QgsExpression dispExpression( layer->displayExpression() );
QgsExpressionContext context;
context.appendScopes( QgsExpressionContextUtils::globalProjectLayerScopes( layer ) );
dispExpression.prepare( &context );
mDispExpression = QgsExpression( layer->displayExpression() );
mContext.appendScopes( QgsExpressionContextUtils::globalProjectLayerScopes( layer ) );
mDispExpression.prepare( &mContext );

// build up request expression
QStringList expressionParts;
@@ -211,17 +249,25 @@ void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, c
req.setFlags( QgsFeatureRequest::NoGeometry );
req.setFilterExpression( expression );
req.setLimit( 30 );
mIterator = layer->getFeatures( req );

mLayerId = layer->id();
mLayerIcon = QgsMapLayerModel::iconForLayer( layer );
}

void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
{
int found = 0;
QgsFeature f;
QgsFeatureIterator it = layer->getFeatures( req );
while ( it.nextFeature( f ) )

while ( mIterator.nextFeature( f ) )
{
if ( feedback->isCanceled() )
return;

QgsLocatorResult result;
result.filter = this;

context.setFeature( f );
mContext.setFeature( f );

// find matching field content
Q_FOREACH ( const QVariant &var, f.attributes() )
@@ -236,10 +282,10 @@ void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, c
if ( result.displayString.isEmpty() )
continue; //not sure how this result slipped through...

result.description = dispExpression.evaluate( &context ).toString();
result.description = mDispExpression.evaluate( &mContext ).toString();

result.userData = QVariantList() << f.id() << layer->id();
result.icon = QgsMapLayerModel::iconForLayer( layer );
result.userData = QVariantList() << f.id() << mLayerId;
result.icon = mLayerIcon;
result.score = static_cast< double >( string.length() ) / result.displayString.size();
emit resultFetched( result );

0 comments on commit 981afb3

Please sign in to comment.
You can’t perform that action at this time.