whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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 ungrapMessageHandler() 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.

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 <thiago.macieira@intel.com>
Marc Mutz 2019-06-21 22:53:14 +02:00
parent 15a1d36b1a
commit cd401b74a1
1 changed files with 19 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/corelib/global/qlogging.cpp
View File

@ -44,6 +44,7 @@
#include "qlogging_p.h"
#include "qlist.h"
#include "qbytearray.h"
#include "qscopeguard.h"
#include "qstring.h"
#include "qvarlengtharray.h"
#include "qdebug.h"
@ -1499,9 +1500,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 -------------------------
@ -1813,14 +1814,15 @@ 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()) {
const auto ungrab = qScopeGuard([]{ ungrabMessageHandler(); });
auto oldStyle = msgHandler.loadAcquire();
auto newStye = messageHandler.loadAcquire();
// prefer new message handler over the old one
if (msgHandler.loadRelaxed() == qDefaultMsgHandler
|| messageHandler.loadRelaxed() != qDefaultMessageHandler) {
(*messageHandler.loadRelaxed())(msgType, context, message);
if (newStye || !oldStyle) {
(newStye ? newStye : qDefaultMessageHandler)(msgType, context, message);
} else {
(*msgHandler.loadRelaxed())(msgType, message.toLocal8Bit().constData());
(oldStyle ? oldStyle : qDefaultMsgHandler)(msgType, message.toLocal8Bit().constData());
}
ungrabMessageHandler();
} else {
fprintf(stderr, "%s\n", message.toLocal8Bit().constData());
}
@ -2071,18 +2073,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)