File 0003-Fix-data-race-on-QLoggingCategory-when-using-qDebug-.patch of Package libqt5-qtbase.9686
From 884b38157689a893ace0a4c793fab921167abb2c Mon Sep 17 00:00:00 2001
From: David Faure <david.faure@kdab.com>
Date: Sun, 8 Jun 2014 21:46:24 +0200
Subject: [PATCH 1/1] Fix data race on QLoggingCategory when using qDebug from
multiple threads
setEnabled() would race with isEnabled()/isDebugEnabled()/etc.
Change-Id: I2004cba81d5417a634b97f5c2f98d3a4ab71770d
Reviewed-by: David Faure <david.faure@kdab.com>
---
src/corelib/arch/qatomic_bootstrap.h | 4 ++-
src/corelib/io/qloggingcategory.cpp | 36 ++++++++++++++++++--------
src/corelib/io/qloggingcategory.h | 35 +++++++++++++++++++------
tests/auto/corelib/io/qdebug/qdebug.pro | 2 +-
tests/auto/corelib/io/qdebug/tst_qdebug.cpp | 40 +++++++++++++++++++++++++++++
5 files changed, 97 insertions(+), 20 deletions(-)
diff --git a/src/corelib/arch/qatomic_bootstrap.h b/src/corelib/arch/qatomic_bootstrap.h
index 7f17387..0d6843a 100644
--- a/src/corelib/arch/qatomic_bootstrap.h
+++ b/src/corelib/arch/qatomic_bootstrap.h
@@ -67,8 +67,10 @@ template <typename T> struct QAtomicOps: QGenericAtomicOps<QAtomicOps<T> >
return --_q_value != 0;
}
- static bool testAndSetRelaxed(T &_q_value, T expectedValue, T newValue) Q_DECL_NOTHROW
+ static bool testAndSetRelaxed(T &_q_value, T expectedValue, T newValue, T *currentValue = 0) Q_DECL_NOTHROW
{
+ if (currentValue)
+ *currentValue = _q_value;
if (_q_value == expectedValue) {
_q_value = newValue;
return true;
diff --git a/src/corelib/io/qloggingcategory.cpp b/src/corelib/io/qloggingcategory.cpp
index 518052e..45fab11 100644
--- a/src/corelib/io/qloggingcategory.cpp
+++ b/src/corelib/io/qloggingcategory.cpp
@@ -50,6 +50,18 @@ const char qtDefaultCategoryName[] = "default";
Q_GLOBAL_STATIC_WITH_ARGS(QLoggingCategory, qtDefaultCategory,
(qtDefaultCategoryName))
+#ifndef Q_ATOMIC_INT8_IS_SUPPORTED
+static void setBoolLane(QBasicAtomicInt *atomic, bool enable, int shift)
+{
+ const int bit = 1 << shift;
+
+ if (enable)
+ atomic->fetchAndOrRelaxed(bit);
+ else
+ atomic->fetchAndAndRelaxed(~bit);
+}
+#endif
+
/*!
\class QLoggingCategory
\inmodule QtCore
@@ -171,13 +183,11 @@ Q_GLOBAL_STATIC_WITH_ARGS(QLoggingCategory, qtDefaultCategory,
*/
QLoggingCategory::QLoggingCategory(const char *category)
: d(0),
- name(0),
- enabledDebug(true),
- enabledWarning(true),
- enabledCritical(true)
+ name(0)
{
Q_UNUSED(d);
Q_UNUSED(placeholder);
+ enabled.store(0x01010101); // enabledDebug = enabledWarning = enabledCritical = true;
const bool isDefaultCategory
= (category == 0) || (strcmp(category, qtDefaultCategoryName) == 0);
@@ -249,9 +259,9 @@ QLoggingCategory::~QLoggingCategory()
bool QLoggingCategory::isEnabled(QtMsgType msgtype) const
{
switch (msgtype) {
- case QtDebugMsg: return enabledDebug;
- case QtWarningMsg: return enabledWarning;
- case QtCriticalMsg: return enabledCritical;
+ case QtDebugMsg: return isDebugEnabled();
+ case QtWarningMsg: return isWarningEnabled();
+ case QtCriticalMsg: return isCriticalEnabled();
case QtFatalMsg: return true;
}
return false;
@@ -270,9 +280,15 @@ bool QLoggingCategory::isEnabled(QtMsgType msgtype) const
void QLoggingCategory::setEnabled(QtMsgType type, bool enable)
{
switch (type) {
- case QtDebugMsg: enabledDebug = enable; break;
- case QtWarningMsg: enabledWarning = enable; break;
- case QtCriticalMsg: enabledCritical = enable; break;
+#ifdef Q_ATOMIC_INT8_IS_SUPPORTED
+ case QtDebugMsg: bools.enabledDebug.store(enable); break;
+ case QtWarningMsg: bools.enabledWarning.store(enable); break;
+ case QtCriticalMsg: bools.enabledCritical.store(enable); break;
+#else
+ case QtDebugMsg: setBoolLane(&enabled, enable, DebugShift); break;
+ case QtWarningMsg: setBoolLane(&enabled, enable, WarningShift); break;
+ case QtCriticalMsg: setBoolLane(&enabled, enable, CriticalShift); break;
+#endif
case QtFatalMsg: break;
}
}
diff --git a/src/corelib/io/qloggingcategory.h b/src/corelib/io/qloggingcategory.h
index 4aec8e6..573af21 100644
--- a/src/corelib/io/qloggingcategory.h
+++ b/src/corelib/io/qloggingcategory.h
@@ -57,10 +57,15 @@ public:
bool isEnabled(QtMsgType type) const;
void setEnabled(QtMsgType type, bool enable);
- bool isDebugEnabled() const { return enabledDebug; }
- bool isWarningEnabled() const { return enabledWarning; }
- bool isCriticalEnabled() const { return enabledCritical; }
-
+#ifdef Q_ATOMIC_INT8_IS_SUPPORTED
+ bool isDebugEnabled() const { return bools.enabledDebug.load(); }
+ bool isWarningEnabled() const { return bools.enabledWarning.load(); }
+ bool isCriticalEnabled() const { return bools.enabledCritical.load(); }
+#else
+ bool isDebugEnabled() const { return enabled.load() >> DebugShift & 1; }
+ bool isWarningEnabled() const { return enabled.load() >> WarningShift & 1; }
+ bool isCriticalEnabled() const { return enabled.load() >> CriticalShift & 1; }
+#endif
const char *categoryName() const { return name; }
// allows usage of both factory method and variable in qCX macros
@@ -78,10 +83,24 @@ private:
void *d; // reserved for future use
const char *name;
- bool enabledDebug;
- bool enabledWarning;
- bool enabledCritical;
- bool placeholder[5]; // reserve for future use
+#ifdef Q_BIG_ENDIAN
+ enum { DebugShift = 0, WarningShift = 8, CriticalShift = 16 };
+#else
+ enum { DebugShift = 24, WarningShift = 16, CriticalShift = 8 };
+#endif
+
+ struct AtomicBools {
+#ifdef Q_ATOMIC_INT8_IS_SUPPORTED
+ QBasicAtomicInteger<bool> enabledDebug;
+ QBasicAtomicInteger<bool> enabledWarning;
+ QBasicAtomicInteger<bool> enabledCritical;
+#endif
+ };
+ union {
+ AtomicBools bools;
+ QBasicAtomicInt enabled;
+ };
+ bool placeholder[4]; // reserve for future use
};
#define Q_DECLARE_LOGGING_CATEGORY(name) \
diff --git a/tests/auto/corelib/io/qdebug/qdebug.pro b/tests/auto/corelib/io/qdebug/qdebug.pro
index 820c17f..5e902bb 100644
--- a/tests/auto/corelib/io/qdebug/qdebug.pro
+++ b/tests/auto/corelib/io/qdebug/qdebug.pro
@@ -1,5 +1,5 @@
CONFIG += testcase parallel_test
TARGET = tst_qdebug
-QT = core testlib
+QT = core testlib concurrent
SOURCES = tst_qdebug.cpp
DEFINES += QT_DISABLE_DEPRECATED_BEFORE=0
diff --git a/tests/auto/corelib/io/qdebug/tst_qdebug.cpp b/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
index 80144db..8fd830a 100644
--- a/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
+++ b/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
@@ -44,6 +44,9 @@
#include <QtCore/QtDebug>
#include <QtTest/QtTest>
+#include <QtConcurrentRun>
+#include <QFutureSynchronizer>
+
class tst_QDebug: public QObject
{
Q_OBJECT
@@ -59,6 +62,7 @@ private slots:
void qDebugQLatin1String() const;
void textStreamModifiers() const;
void defaultMessagehandler() const;
+ void threadSafety() const;
};
void tst_QDebug::assignment() const
@@ -305,5 +309,41 @@ void tst_QDebug::defaultMessagehandler() const
QVERIFY(same);
}
+QMutex s_mutex;
+QStringList s_messages;
+QSemaphore s_sema;
+
+static void threadSafeMessageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg)
+{
+ QMutexLocker lock(&s_mutex);
+ s_messages.append(msg);
+ Q_UNUSED(type);
+ Q_UNUSED(context);
+}
+
+static void doDebug() // called in each thread
+{
+ s_sema.acquire();
+ qDebug() << "doDebug";
+}
+
+void tst_QDebug::threadSafety() const
+{
+ MessageHandlerSetter mhs(threadSafeMessageHandler);
+ const int numThreads = 10;
+ QThreadPool::globalInstance()->setMaxThreadCount(numThreads);
+ QFutureSynchronizer<void> sync;
+ for (int i = 0; i < numThreads; ++i) {
+ sync.addFuture(QtConcurrent::run(&doDebug));
+ }
+ s_sema.release(numThreads);
+ sync.waitForFinished();
+ QMutexLocker lock(&s_mutex);
+ QCOMPARE(s_messages.count(), numThreads);
+ for (int i = 0; i < numThreads; ++i) {
+ QCOMPARE(s_messages.at(i), QStringLiteral("doDebug"));
+ }
+}
+
QTEST_MAIN(tst_QDebug);
#include "tst_qdebug.moc"
--
1.9.3
