From 9ee27005ee3cfa295c681a5ac04ac1c282b396ca Mon Sep 17 00:00:00 2001
From: David Faure <david.faure@kdab.com>
Date: Sun, 8 Jun 2014 14:10:56 +0200
Subject: qDebug: fix data race in qt_message_print

The setting of (static) messageHandler to qDefaultMessageHandler if null
was happening in multiple threads simultaneously, so it needs synchronization.

Used an atomic pointer in case qInstallMessageHandler is called from a thread,
but more importantly, initialized the static vars right away.

Improve auto test to ensure that qInstallMessageHandler(0) still sets the
default message handler.

Change-Id: I70335af38c1d28a1cdba1df8a79c6006f227422e
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/global/qlogging.cpp             | 42 +++++++++++++----------------
 tests/auto/corelib/io/qdebug/tst_qdebug.cpp |  9 ++++---
 2 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/src/corelib/global/qlogging.cpp b/src/corelib/global/qlogging.cpp
index ff4372a049..ca0fb1bb23 100644
--- a/src/corelib/global/qlogging.cpp
+++ b/src/corelib/global/qlogging.cpp
@@ -1148,8 +1148,13 @@ typedef void (*QtMsgHandler)(QtMsgType, const char *);
 Q_CORE_EXPORT QtMsgHandler qInstallMsgHandler(QtMsgHandler);
 #endif
 
-static QtMsgHandler msgHandler = 0;                // pointer to debug handler (without context)
-static QtMessageHandler messageHandler = 0;         // pointer to debug handler (with context)
+static void qDefaultMsgHandler(QtMsgType type, const char *buf);
+static void qDefaultMessageHandler(QtMsgType type, const QMessageLogContext &context, const QString &buf);
+
+// pointer to QtMsgHandler debug handler (without context)
+static QBasicAtomicPointer<void (QtMsgType, const char*)> msgHandler = Q_BASIC_ATOMIC_INITIALIZER(qDefaultMsgHandler);
+// pointer to QtMessageHandler debug handler (with context)
+static QBasicAtomicPointer<void (QtMsgType, const QMessageLogContext &, const QString &)> messageHandler = Q_BASIC_ATOMIC_INITIALIZER(qDefaultMessageHandler);
 
 #if defined(QT_USE_JOURNALD) && !defined(QT_BOOTSTRAPPED)
 static void systemd_default_message_handler(QtMsgType type,
@@ -1315,20 +1320,15 @@ static void qt_message_print(QtMsgType msgType, const QMessageLogContext &contex
     }
 #endif
 
-    if (!msgHandler)
-        msgHandler = qDefaultMsgHandler;
-    if (!messageHandler)
-        messageHandler = qDefaultMessageHandler;
-
     // prevent recursion in case the message handler generates messages
     // itself, e.g. by using Qt API
     if (grabMessageHandler()) {
         // prefer new message handler over the old one
-        if (msgHandler == qDefaultMsgHandler
-                || messageHandler != qDefaultMessageHandler) {
-            (*messageHandler)(msgType, context, message);
+        if (msgHandler.load() == qDefaultMsgHandler
+                || messageHandler.load() != qDefaultMessageHandler) {
+            (*messageHandler.load())(msgType, context, message);
         } else {
-            (*msgHandler)(msgType, message.toLocal8Bit().constData());
+            (*msgHandler.load())(msgType, message.toLocal8Bit().constData());
         }
         ungrabMessageHandler();
     } else {
@@ -1530,22 +1530,18 @@ void qErrnoWarning(int code, const char *msg, ...)
 
 QtMessageHandler qInstallMessageHandler(QtMessageHandler h)
 {
-    if (!messageHandler)
-        messageHandler = qDefaultMessageHandler;
-    QtMessageHandler old = messageHandler;
-    messageHandler = h;
-    return old;
+    if (!h)
+        h = qDefaultMessageHandler;
+    //set 'h' and return old message handler
+    return messageHandler.fetchAndStoreRelaxed(h);
 }
 
 QtMsgHandler qInstallMsgHandler(QtMsgHandler h)
 {
-    //if handler is 0, set it to the
-    //default message handler
-    if (!msgHandler)
-        msgHandler = qDefaultMsgHandler;
-    QtMsgHandler old = msgHandler;
-    msgHandler = h;
-    return old;
+    if (!h)
+        h = qDefaultMsgHandler;
+    //set 'h' and return old message handler
+    return msgHandler.fetchAndStoreRelaxed(h);
 }
 
 void qSetMessagePattern(const QString &pattern)
diff --git a/tests/auto/corelib/io/qdebug/tst_qdebug.cpp b/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
index 8fd830a839..99c4ee7edc 100644
--- a/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
+++ b/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
@@ -299,12 +299,13 @@ void tst_QDebug::textStreamModifiers() const
 
 void tst_QDebug::defaultMessagehandler() const
 {
-    MessageHandlerSetter mhs(0);
-    QtMessageHandler defaultMessageHandler1 = qInstallMessageHandler((QtMessageHandler)0);
-    QtMessageHandler defaultMessageHandler2 = qInstallMessageHandler(myMessageHandler);
+    MessageHandlerSetter mhs(0); // set 0, should set default handler
+    QtMessageHandler defaultMessageHandler1 = qInstallMessageHandler((QtMessageHandler)0); // set 0, should set and return default handler
+    QVERIFY(defaultMessageHandler1);
+    QtMessageHandler defaultMessageHandler2 = qInstallMessageHandler(myMessageHandler); // set myMessageHandler and return default handler
     bool same = (*defaultMessageHandler1 == *defaultMessageHandler2);
     QVERIFY(same);
-    QtMessageHandler messageHandler = qInstallMessageHandler((QtMessageHandler)0);
+    QtMessageHandler messageHandler = qInstallMessageHandler((QtMessageHandler)0); // set 0, should set default and return myMessageHandler
     same = (*messageHandler == *myMessageHandler);
     QVERIFY(same);
 }
-- 
cgit v1.2.3