diff options
authorMarc Mutz <>2019-06-21 22:53:14 +0200
committerMarc Mutz <>2019-07-11 20:05:19 +0000
commit987dde2965f4fba32984041a0f2709127e9edd93 (patch)
parent54684f10e90b2ab1705cad97dd0a5d3942f06cc3 (diff)
Optimize and fix handling of QtMessageHandlers
A function may almost always have static storage duration, but that does not necessarily mean that we can store and load pointers to them without memory ordering. Play it safe and use store-release and load-acquire for them (which combines to ordered for the fetchAndSet call in qInstall*Handler(), as we don't know what the caller will do with the returned function pointer). Also change the initial value of the atomic pointer to nullptr. Nullptr already signified the default handler in qInstall*Handler(), so the API doesn't change. But by using nullptr to mean default, we place these variables in the BSS segment instead of TEXT, save dynamic init, or at least a relocation, and we dodge the smelly comparison of function pointers, using comparison against nullptr instead. Also, as a drive-by, put the call to ungrabMessageHandler() in a scope-guard. Both the message handler, as well as the Qt code calling it (toLocal8Bit()!), may throw, and that would stop all further logging. In Qt 5.9, we can't use qScopeGuard(), yet, so use a local struct calling ungrabMessageHandler() in its dtor. The code still has one problem: When a logging action is underway, and another thread exchanges the message handler, we might still execute code in the old handler. This is probably not a problem in practice, since no-one will use a dynamically-compiled function for logging (right? :), but should probably be documented or fixed. This patch does not address this issue, though. Change-Id: I21aa907288b9c8c6646787b4001002d145b114a5 Reviewed-by: Thiago Macieira <> (cherry picked from commit cd401b74a13cd9d9a47d977f195c7985cf725d55) (cherry picked from commit ea16c860bd75a35134ebb1d4f3be5db58f4a4e21)
1 files changed, 20 insertions, 15 deletions
diff --git a/src/corelib/global/qlogging.cpp b/src/corelib/global/qlogging.cpp
index 30232170fb..1a74757032 100644
--- a/src/corelib/global/qlogging.cpp
+++ b/src/corelib/global/qlogging.cpp
@@ -1512,9 +1512,9 @@ 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);
+static QBasicAtomicPointer<void (QtMsgType, const char*)> msgHandler = Q_BASIC_ATOMIC_INITIALIZER(nullptr);
// pointer to QtMessageHandler debug handler (with context)
-static QBasicAtomicPointer<void (QtMsgType, const QMessageLogContext &, const QString &)> messageHandler = Q_BASIC_ATOMIC_INITIALIZER(qDefaultMessageHandler);
+static QBasicAtomicPointer<void (QtMsgType, const QMessageLogContext &, const QString &)> messageHandler = Q_BASIC_ATOMIC_INITIALIZER(nullptr);
// ------------------------ Alternate logging sinks -------------------------
@@ -1826,14 +1826,17 @@ static void qt_message_print(QtMsgType msgType, const QMessageLogContext &contex
// prevent recursion in case the message handler generates messages
// itself, e.g. by using Qt API
if (grabMessageHandler()) {
+ struct Ungrabber {
+ ~Ungrabber() { ungrabMessageHandler(); }
+ } ungrabber;
+ auto oldStyle = msgHandler.loadAcquire();
+ auto newStye = messageHandler.loadAcquire();
// prefer new message handler over the old one
- if (msgHandler.load() == qDefaultMsgHandler
- || messageHandler.load() != qDefaultMessageHandler) {
- (*messageHandler.load())(msgType, context, message);
+ if (newStye || !oldStyle) {
+ (newStye ? newStye : qDefaultMessageHandler)(msgType, context, message);
} else {
- (*msgHandler.load())(msgType, message.toLocal8Bit().constData());
+ (oldStyle ? oldStyle : qDefaultMsgHandler)(msgType, message.toLocal8Bit().constData());
- ungrabMessageHandler();
} else {
fprintf(stderr, "%s\n", message.toLocal8Bit().constData());
@@ -2084,18 +2087,20 @@ void qErrnoWarning(int code, const char *msg, ...)
QtMessageHandler qInstallMessageHandler(QtMessageHandler h)
- if (!h)
- h = qDefaultMessageHandler;
- //set 'h' and return old message handler
- return messageHandler.fetchAndStoreRelaxed(h);
+ const auto old = messageHandler.fetchAndStoreOrdered(h);
+ if (old)
+ return old;
+ else
+ return qDefaultMessageHandler;
QtMsgHandler qInstallMsgHandler(QtMsgHandler h)
- if (!h)
- h = qDefaultMsgHandler;
- //set 'h' and return old message handler
- return msgHandler.fetchAndStoreRelaxed(h);
+ const auto old = msgHandler.fetchAndStoreOrdered(h);
+ if (old)
+ return old;
+ else
+ return qDefaultMsgHandler;
void qSetMessagePattern(const QString &pattern)