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>

2 files changed, 16 insertions, 7 deletions

diff --git a/src/corelib/io/qloggingregistry.cpp b/src/corelib/io/qloggingregistry.cpp
index eaebbc1ffc..77a8c891b4 100644
--- a/src/corelib/io/qloggingregistry.cpp
+++ b/src/corelib/io/qloggingregistry.cpp
@@ -276,10 +276,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()) {
@@ -287,7 +288,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");
@@ -296,17 +297,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 = std::move(er);
+    qtConfigRules = std::move(qr);
+    configRules = std::move(cr);
 
     if (!envRules.isEmpty() || !qtConfigRules.isEmpty() || !configRules.isEmpty()) {
-        QMutexLocker locker(&registryMutex);
         updateRules();
     }
 }
@@ -347,11 +353,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();
@@ -400,6 +406,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 5197da1ba4..23740c4955 100644
--- a/src/corelib/io/qloggingregistry_p.h
+++ b/src/corelib/io/qloggingregistry_p.h
@@ -129,6 +129,7 @@ private:
 
     QMutex registryMutex;
 
+    // protected by mutex:
     QVector<QLoggingRule> qtConfigRules;
     QVector<QLoggingRule> configRules;
     QVector<QLoggingRule> envRules;