From 567abeaa04db2c30f0d9a5cb95e11d295ad88784 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sat, 18 Feb 2017 09:06:00 +0100 Subject: 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) --- src/corelib/io/qloggingregistry.cpp | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'src/corelib/io/qloggingregistry.cpp') 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 loadRulesFromFile(const QString &filePath) */ void QLoggingRegistry::init() { + QVector 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(®istryMutex); + + envRules = std::move(er); + qtConfigRules = std::move(qr); + configRules = std::move(cr); if (!envRules.isEmpty() || !qtConfigRules.isEmpty() || !configRules.isEmpty()) { - QMutexLocker locker(®istryMutex); updateRules(); } } @@ -347,11 +353,11 @@ void QLoggingRegistry::setApiRules(const QString &content) parser.setSection(QStringLiteral("Rules")); parser.setContent(content); - QMutexLocker locker(®istryMutex); - if (qtLoggingDebug()) debugMsg("Loading logging rules set by QLoggingCategory::setFilterRules ..."); + const QMutexLocker locker(®istryMutex); + 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) { -- cgit v1.2.3