diff options
| author | Thiago Macieira <thiago.macieira@intel.com> | 2025-02-08 14:33:27 -0800 |
|---|---|---|
| committer | Thiago Macieira <thiago.macieira@intel.com> | 2025-03-31 17:04:13 -0400 |
| commit | 4afd089612448ae13136e287ced134c71499cfec (patch) | |
| tree | 0215f40365e84069243c55547fdf78508c3c148b | |
| parent | 51c8c0695192677f821759b98f8ba6496df41fa7 (diff) | |
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 <edward.welbourne@qt.io>
| -rw-r--r-- | src/corelib/CMakeLists.txt | 2 | ||||
| -rw-r--r-- | src/corelib/global/qlogging.cpp | 2 | ||||
| -rw-r--r-- | src/corelib/io/qloggingcategory.cpp | 22 | ||||
| -rw-r--r-- | src/corelib/io/qloggingcategory.h | 4 | ||||
| -rw-r--r-- | src/corelib/io/qloggingcategory_p.h | 35 | ||||
| -rw-r--r-- | src/corelib/io/qloggingregistry.cpp | 23 | ||||
| -rw-r--r-- | src/corelib/io/qloggingregistry_p.h | 7 |
7 files changed, 76 insertions, 19 deletions
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 <QtCore/qloggingcategory.h> +#include <QtCore/private/qglobal_p.h> + +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<QLoggingCategory *>(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 <QtCore/private/qglobal_p.h> -#include <QtCore/qloggingcategory.h> +#include <QtCore/private/qloggingcategory_p.h> #include <QtCore/qlist.h> #include <QtCore/qhash.h> #include <QtCore/qmutex.h> @@ -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); |