QLoggingRegistry: fix potential data race

The 'rules' vector is made up of all the individual {env,config,...}Rules
vectors under mutex protection whenever init() is called (only from the
QCoreApplication ctor) or, at any time, by a call to QLoggingCategory::
setFilterRules().

Yet, the writes to the individual *Rules vectors were never protected by
registryMutex, racing against the reads of the same vectors in the
updateRules() function.

Fix by protecting all access of all member variables with registryMutex.
Add some strategic comments to make analysis easier for the next guy.

Change-Id: If68d15a553ec7038693574a34f10a39f4cd480e8
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
(cherry picked from commit 567abeaa04db2c30f0d9a5cb95e11d295ad88784)
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

2 files changed, 16 insertions, 7 deletions

diff --git a/src/corelib/io/qloggingregistry.cpp b/src/corelib/io/qloggingregistry.cpp
index a9ac38c6a6..1851aace6a 100644
--- a/src/corelib/io/qloggingregistry.cpp
+++ b/src/corelib/io/qloggingregistry.cpp
@@ -270,10 +270,11 @@ static QVector<QLoggingRule> loadRulesFromFile(const QString &filePath)
  */
 void QLoggingRegistry::init()
 {
+    QVector<QLoggingRule> er, qr, cr;
     // get rules from environment
     const QByteArray rulesFilePath = qgetenv("QT_LOGGING_CONF");
     if (!rulesFilePath.isEmpty())
-        envRules = loadRulesFromFile(QFile::decodeName(rulesFilePath));
+        er = loadRulesFromFile(QFile::decodeName(rulesFilePath));
 
     const QByteArray rulesSrc = qgetenv("QT_LOGGING_RULES").replace(';', '\n');
     if (!rulesSrc.isEmpty()) {
@@ -281,7 +282,7 @@ void QLoggingRegistry::init()
          QLoggingSettingsParser parser;
          parser.setSection(QStringLiteral("Rules"));
          parser.setContent(stream);
-         envRules += parser.rules();
+         er += parser.rules();
     }
 
     const QString configFileName = QStringLiteral("qtlogging.ini");
@@ -290,17 +291,22 @@ void QLoggingRegistry::init()
     // get rules from Qt data configuration path
     const QString qtConfigPath
             = QDir(QLibraryInfo::location(QLibraryInfo::DataPath)).absoluteFilePath(configFileName);
-    qtConfigRules = loadRulesFromFile(qtConfigPath);
+    qr = loadRulesFromFile(qtConfigPath);
 #endif
 
     // get rules from user's/system configuration
     const QString envPath = QStandardPaths::locate(QStandardPaths::GenericConfigLocation,
                                                    QString::fromLatin1("QtProject/") + configFileName);
     if (!envPath.isEmpty())
-        configRules = loadRulesFromFile(envPath);
+        cr = loadRulesFromFile(envPath);
+
+    const QMutexLocker locker(&registryMutex);
+
+    envRules = qMove(er);
+    qtConfigRules = qMove(qr);
+    configRules = qMove(cr);
 
     if (!envRules.isEmpty() || !qtConfigRules.isEmpty() || !configRules.isEmpty()) {
-        QMutexLocker locker(&registryMutex);
         updateRules();
     }
 }
@@ -341,11 +347,11 @@ void QLoggingRegistry::setApiRules(const QString &content)
     parser.setSection(QStringLiteral("Rules"));
     parser.setContent(content);
 
-    QMutexLocker locker(&registryMutex);
-
     if (qtLoggingDebug())
         debugMsg("Loading logging rules set by QLoggingCategory::setFilterRules ...");
 
+    const QMutexLocker locker(&registryMutex);
+
     apiRules = parser.rules();
 
     updateRules();
@@ -394,6 +400,8 @@ QLoggingRegistry *QLoggingRegistry::instance()
 /*!
     \internal
     Updates category settings according to rules.
+
+    As a category filter, it is run with registryMutex held.
 */
 void QLoggingRegistry::defaultCategoryFilter(QLoggingCategory *cat)
 {
diff --git a/src/corelib/io/qloggingregistry_p.h b/src/corelib/io/qloggingregistry_p.h
index bb32a4996d..c3b49666c9 100644
--- a/src/corelib/io/qloggingregistry_p.h
+++ b/src/corelib/io/qloggingregistry_p.h
@@ -122,6 +122,7 @@ private:
 
     QMutex registryMutex;
 
+    // protected by mutex:
     QVector<QLoggingRule> qtConfigRules;
     QVector<QLoggingRule> configRules;
     QVector<QLoggingRule> envRules;