Skip to content

Commit 69faab9

Browse files
rouaultm-kuhn
authored andcommitted
Fix thread-unsafe initialization of QgsExpression::Functions()
The method initializes the gmFunctions static member, without any mutex protection. This turned out to cause random crashes in the tests of the WFS provider since the downloader thread may evaluate an expression, in parallel of the main thread, which does the same. This was mainly seen on Mac Travis (2 crashes + 1 failures, over 50 iterations), when parallelizing tests so as to get particular scheduling : https://travis-ci.org/rouault/Quantum-GIS/builds/121720556. But I could finally reproduce it systematically on my Linux box when inserting the following sleep. diff --git a/src/providers/wfs/qgswfsshareddata.cpp b/src/providers/wfs/qgswfsshareddata.cpp index adc7042..e9e4577 100644 --- a/src/providers/wfs/qgswfsshareddata.cpp +++ b/src/providers/wfs/qgswfsshareddata.cpp @@ -426,6 +426,7 @@ int QgsWFSSharedData::registerToCache( QgsWFSFeatureIterator* iterator, QgsRecta connect( mDownloader, SIGNAL( ready() ), &loop, SLOT( quit() ) ); mDownloader->start(); loop.exec( QEventLoop::ExcludeUserInputEvents ); + usleep( 100 * 1000 ); } if ( mDownloadFinished ) return -1; After applying this commit, the Mac builder is fine: https://travis-ci.org/rouault/Quantum-GIS/builds/121756158
1 parent 0aed3f7 commit 69faab9

File tree

1 file changed

+8
-0
lines changed

1 file changed

+8
-0
lines changed

src/core/qgsexpression.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <QRegExp>
2222
#include <QColor>
2323
#include <QUuid>
24+
#include <QMutex>
2425

2526
#include <math.h>
2627
#include <limits>
@@ -1694,6 +1695,13 @@ QList<QgsExpression::Function*> QgsExpression::gmFunctions;
16941695

16951696
const QList<QgsExpression::Function*> &QgsExpression::Functions()
16961697
{
1698+
// The construction of the list isn't thread-safe, and without the mutex,
1699+
// crashes in the WFS provider may occur, since it can parse expressions
1700+
// in parallel.
1701+
// The mutex needs to be recursive.
1702+
static QMutex sFunctionsMutex( QMutex::Recursive );
1703+
QMutexLocker locker( &sFunctionsMutex );
1704+
16971705
if ( gmFunctions.isEmpty() )
16981706
{
16991707
gmFunctions

0 commit comments

Comments
 (0)