From a47d974e19512ae5a445f64fbd2b1703201f781d Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Thu, 16 Feb 2012 23:49:27 +0100 Subject: QRegularExpression: fix optimizePattern, document the issue The studyData pointer is atomically set by the pointer assignment, but another processor running a different thread might see the new studyData value but not the memory it points to. Therefore, the current studyData is returned from optimizePattern and used by that thread. Docs were added to optimizePattern to explain what's going on. Change-Id: I4502c336077bb98a1751011aa93ffd4f585ed101 Reviewed-by: Thiago Macieira --- src/corelib/tools/qregularexpression.cpp | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/corelib/tools/qregularexpression.cpp b/src/corelib/tools/qregularexpression.cpp index 7faa907e35..0252a30c89 100644 --- a/src/corelib/tools/qregularexpression.cpp +++ b/src/corelib/tools/qregularexpression.cpp @@ -773,7 +773,7 @@ struct QRegularExpressionPrivate : QSharedData void cleanCompiledPattern(); void compilePattern(); void getPatternInfo(); - void optimizePattern(); + pcre16_extra *optimizePattern(); QRegularExpressionMatchPrivate *doMatch(const QString &subject, int offset, @@ -1006,15 +1006,30 @@ static bool isJitEnabled() /*! \internal + + The purpose of the function is to call pcre16_study (which allows some + optimizations to be performed, including JIT-compiling the pattern), and + setting the studyData member variable to the result of the study. It gets + called by doMatch() every time a match is performed. As of now, the + optimizations on the pattern are performed after a certain number of usages + (i.e. the OPTIMIZE_AFTER_USE_COUNT constant). + + Notice that although the method is protected by a mutex, one thread may + invoke this function and return immediately (i.e. not study the pattern, + leaving studyData to NULL); but before calling pcre16_exec to perform the + match, another thread performs the studying and sets studyData to something + else. Although the assignment to studyData is itself atomic, the release of + the memory pointed by studyData isn't. Therefore, the current studyData + value is returned and used by doMatch. */ -void QRegularExpressionPrivate::optimizePattern() +pcre16_extra *QRegularExpressionPrivate::optimizePattern() { Q_ASSERT(compiledPattern); QMutexLocker lock(&mutex); if (studyData || (++usedCount != OPTIMIZE_AFTER_USE_COUNT)) - return; + return studyData; static const bool enableJit = isJitEnabled(); @@ -1027,6 +1042,8 @@ void QRegularExpressionPrivate::optimizePattern() if (!studyData && err) qWarning("QRegularExpressionPrivate::optimizePattern(): pcre_study failed: %s", err); + + return studyData; } /*! @@ -1089,7 +1106,7 @@ QRegularExpressionMatchPrivate *QRegularExpressionPrivate::doMatch(const QString capturingCount); // this is mutex protected - const_cast(this)->optimizePattern(); + const pcre16_extra *currentStudyData = const_cast(this)->optimizePattern(); int pcreOptions = convertToPcreOptions(matchOptions); @@ -1113,12 +1130,12 @@ QRegularExpressionMatchPrivate *QRegularExpressionPrivate::doMatch(const QString int result; if (!previousMatchWasEmpty) { - result = pcre16_exec(compiledPattern, studyData, + result = pcre16_exec(compiledPattern, currentStudyData, subjectUtf16, subjectLength, offset, pcreOptions, captureOffsets, captureOffsetsCount); } else { - result = pcre16_exec(compiledPattern, studyData, + result = pcre16_exec(compiledPattern, currentStudyData, subjectUtf16, subjectLength, offset, pcreOptions | PCRE_NOTEMPTY_ATSTART | PCRE_ANCHORED, captureOffsets, captureOffsetsCount); @@ -1136,7 +1153,7 @@ QRegularExpressionMatchPrivate *QRegularExpressionPrivate::doMatch(const QString ++offset; } - result = pcre16_exec(compiledPattern, studyData, + result = pcre16_exec(compiledPattern, currentStudyData, subjectUtf16, subjectLength, offset, pcreOptions, captureOffsets, captureOffsetsCount); -- cgit v1.2.3