From 4afd089612448ae13136e287ced134c71499cfec Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 8 Feb 2025 14:33:27 -0800 Subject: [PATCH] QLoggingCategory: make defaultCategory() truly a global static We don't need two independent Q_GLOBAL_STATIC variables: one for the default category and the other for the logging registry. We can simply save one in the other. However, we can do better than even that. QLoggingCategory currently doesn't hold any allocated resources: the const char *name points to a static string and the void *d is always null. That means the constructor and destructor only serve to register and unregister the category with QLoggingRegistry, so we transfer that responsibility to QLoggingRegistry itself (the un-registering is implicit in the registry's own destruction). The benefit of this is that the default QLoggingCategory can never be found in a destroyed state, however late user code may be running after ::exit() was called. We have had a number of these issues of De-Initialization Order Fiasco, especially since the changes to QThreadData destruction timing. Pick-to: 6.9 Change-Id: I2b6ea597ac257837669afffde2684c7b44a42e92 Reviewed-by: Edward Welbourne --- src/corelib/CMakeLists.txt | 2 +- src/corelib/global/qlogging.cpp | 2 +- src/corelib/io/qloggingcategory.cpp | 22 +++++++----------- src/corelib/io/qloggingcategory.h | 4 ++++ src/corelib/io/qloggingcategory_p.h | 35 +++++++++++++++++++++++++++++ src/corelib/io/qloggingregistry.cpp | 23 ++++++++++++++++++- src/corelib/io/qloggingregistry_p.h | 7 ++++-- 7 files changed, 76 insertions(+), 19 deletions(-) create mode 100644 src/corelib/io/qloggingcategory_p.h diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index b2d51349a93..96e385a8a72 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -135,7 +135,7 @@ qt_internal_add_module(Core io/qiodevicebase.h io/qipaddress.cpp io/qipaddress_p.h io/qlockfile.cpp io/qlockfile.h io/qlockfile_p.h - io/qloggingcategory.cpp io/qloggingcategory.h + io/qloggingcategory.cpp io/qloggingcategory.h io/qloggingcategory_p.h io/qloggingregistry.cpp io/qloggingregistry_p.h io/qnoncontiguousbytedevice.cpp io/qnoncontiguousbytedevice_p.h io/qresource.cpp io/qresource.h io/qresource_p.h diff --git a/src/corelib/global/qlogging.cpp b/src/corelib/global/qlogging.cpp index 12d9422daad..97c7e70e386 100644 --- a/src/corelib/global/qlogging.cpp +++ b/src/corelib/global/qlogging.cpp @@ -953,7 +953,7 @@ QDebug QMessageLogger::fatal(QMessageLogger::CategoryFunction catFunc) const static bool isDefaultCategory(const char *category) { - return !category || strcmp(category, "default") == 0; + return !category || strcmp(category, QLoggingRegistry::defaultCategoryName) == 0; } /*! diff --git a/src/corelib/io/qloggingcategory.cpp b/src/corelib/io/qloggingcategory.cpp index d60abba70d5..4ce2bf0c091 100644 --- a/src/corelib/io/qloggingcategory.cpp +++ b/src/corelib/io/qloggingcategory.cpp @@ -2,13 +2,11 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only #include "qloggingcategory.h" +#include "qloggingcategory_p.h" #include "qloggingregistry_p.h" QT_BEGIN_NAMESPACE -const char qtDefaultCategoryName[] = "default"; -Q_GLOBAL_STATIC(QLoggingCategory, qtDefaultCategory, qtDefaultCategoryName) - /*! \class QLoggingCategory \inmodule QtCore @@ -170,17 +168,10 @@ Q_GLOBAL_STATIC(QLoggingCategory, qtDefaultCategory, qtDefaultCategoryName) \since 5.4 */ QLoggingCategory::QLoggingCategory(const char *category, QtMsgType enableForLevel) - : d(nullptr), - name(nullptr) + : QLoggingCategory(UnregisteredInitialization{}, + category ? category : QLoggingRegistry::defaultCategoryName) { - enabled.storeRelaxed(0x01010101); // enabledDebug = enabledWarning = enabledCritical = true; - - if (category) - name = category; - else - name = qtDefaultCategoryName; - - if (QLoggingRegistry *reg = QLoggingRegistry::instance()) + if (QLoggingRegistry *reg = QLoggingRegistry::instance()) reg->registerCategory(this, enableForLevel); } @@ -189,6 +180,9 @@ QLoggingCategory::QLoggingCategory(const char *category, QtMsgType enableForLeve */ QLoggingCategory::~QLoggingCategory() { + // Note: this destructor is never called for the defaultCategory(), so it + // is always registered. If we ever need to free memory, make + // QLoggingRegistry do it. if (QLoggingRegistry *reg = QLoggingRegistry::instance()) reg->unregisterCategory(this); } @@ -309,7 +303,7 @@ void QLoggingCategory::setEnabled(QtMsgType type, bool enable) */ QLoggingCategory *QLoggingCategory::defaultCategory() { - return qtDefaultCategory(); + return QLoggingRegistry::defaultCategory(); } /*! diff --git a/src/corelib/io/qloggingcategory.h b/src/corelib/io/qloggingcategory.h index f2fdee24e7b..5a4ecf59e33 100644 --- a/src/corelib/io/qloggingcategory.h +++ b/src/corelib/io/qloggingcategory.h @@ -52,6 +52,10 @@ private: QBasicAtomicInt enabled; }; Q_DECL_UNUSED_MEMBER bool placeholder[4]; // reserved for future use + + QT_DEFINE_TAG_STRUCT(UnregisteredInitialization); + explicit constexpr QLoggingCategory(UnregisteredInitialization, const char *category) noexcept; + friend class QLoggingRegistry; }; namespace { // allow different TUs to have different QT_NO_xxx_OUTPUT diff --git a/src/corelib/io/qloggingcategory_p.h b/src/corelib/io/qloggingcategory_p.h new file mode 100644 index 00000000000..46da266db0a --- /dev/null +++ b/src/corelib/io/qloggingcategory_p.h @@ -0,0 +1,35 @@ +// Copyright (C) 2025 Intel Corporation. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only + +#ifndef QLOGGINGCATEGORY_P_H +#define QLOGGINGCATEGORY_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists for the convenience +// of a number of Qt sources files. This header file may change from +// version to version without notice, or even be removed. +// +// We mean it. +// + +#include +#include + +QT_BEGIN_NAMESPACE + +// enabledDebug = enabledWarning = enabledCritical = enableInfo = true; +static constexpr int DefaultLoggingCategoryEnabledValue = 0x01010101; + +constexpr inline +QLoggingCategory::QLoggingCategory(UnregisteredInitialization, const char *category) noexcept + : name(category), + enabled(DefaultLoggingCategoryEnabledValue), + placeholder{} +{} + +QT_END_NAMESPACE + +#endif // QLOGGINGCATEGORY_P_H diff --git a/src/corelib/io/qloggingregistry.cpp b/src/corelib/io/qloggingregistry.cpp index da8ed80ff16..ece58cfa87e 100644 --- a/src/corelib/io/qloggingregistry.cpp +++ b/src/corelib/io/qloggingregistry.cpp @@ -27,6 +27,7 @@ QT_BEGIN_NAMESPACE using namespace Qt::StringLiterals; Q_GLOBAL_STATIC(QLoggingRegistry, qtLoggingRegistry) +alignas(QLoggingCategory) static unsigned char defaultLoggingCategory[sizeof(QLoggingCategory)]; /*! \internal @@ -227,6 +228,14 @@ void QLoggingSettingsParser::parseNextLine(QStringView line) QLoggingRegistry::QLoggingRegistry() : categoryFilter(defaultCategoryFilter) { + using U = QLoggingCategory::UnregisteredInitialization; + Q_ASSERT_X(!self, "QLoggingRegistry", "Singleton recreated"); + self = this; + + // can't use std::construct_at here - private constructor + auto cat = new (defaultLoggingCategory) QLoggingCategory(U{}, defaultCategoryName); + categories.emplace(cat, QtDebugMsg); + #if defined(Q_OS_ANDROID) // Unless QCoreApplication has been constructed we can't be sure that // we are on Qt's main thread. If we did allow logging here, we would @@ -424,6 +433,18 @@ QLoggingRegistry *QLoggingRegistry::instance() return qtLoggingRegistry(); } +QLoggingCategory *QLoggingRegistry::defaultCategory() +{ + // Initialize the defaultLoggingCategory global static, if necessary. Note + // how it remains initialized forever, even if the QLoggingRegistry + // instance() is destroyed. + instance(); + + // std::launder() to be on the safe side, but it's unnecessary because the + // object is never recreated. + return std::launder(reinterpret_cast(defaultLoggingCategory)); +} + /*! \internal Updates category settings according to rules. @@ -432,7 +453,7 @@ QLoggingRegistry *QLoggingRegistry::instance() */ void QLoggingRegistry::defaultCategoryFilter(QLoggingCategory *cat) { - const QLoggingRegistry *reg = QLoggingRegistry::instance(); + const QLoggingRegistry *reg = self; Q_ASSERT(reg->categories.contains(cat)); QtMsgType enableForLevel = reg->categories.value(cat); diff --git a/src/corelib/io/qloggingregistry_p.h b/src/corelib/io/qloggingregistry_p.h index 28d64b7cf2c..0716a33ebda 100644 --- a/src/corelib/io/qloggingregistry_p.h +++ b/src/corelib/io/qloggingregistry_p.h @@ -15,8 +15,7 @@ // We mean it. // -#include -#include +#include #include #include #include @@ -116,8 +115,12 @@ public: Q_CORE_EXPORT static QLoggingRegistry *instance(); + static constexpr const char defaultCategoryName[] = "default"; + static QLoggingCategory *defaultCategory(); + private: Q_AUTOTEST_EXPORT void updateRules(); + static inline QLoggingRegistry *self = nullptr; static void defaultCategoryFilter(QLoggingCategory *category);