From b3f39fdd556be62a277224ba1045caed6affc423 Mon Sep 17 00:00:00 2001 From: David Faure Date: Sun, 9 Mar 2025 14:02:05 +0100 Subject: [PATCH] QRhi: fix data race on rubLogEnabled * Handle QSG_INFO with the proper macro rather than with an invalid const_cast * Remove the racy bool rubLogEnabled, read the atomic from the logging category directly WARNING: ThreadSanitizer: data race (pid=427588) Write of size 1 at 0x7f89d5be2530 by thread T12: #0 QRhiImplementation::prepareForCreate(QRhi*, ...) qrhi.cpp:8863 Previous read of size 1 at 0x7f89d5be2530 by thread T11: #0 QRhiResourceUpdateBatchPrivate::free() qrhi.cpp:9907 Pick-to: 6.8 Change-Id: Ied5171ea5bb97372daced8afe3ad894d49961069 Reviewed-by: Laszlo Agocs (cherry picked from commit 3cc947e98f6237c86160589143e69c13c965e226) --- src/gui/rhi/qrhi.cpp | 27 ++++++++++----------------- src/gui/rhi/qrhi_p.h | 6 +++--- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index 40945b1b5ad..21175ecedb2 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -4,6 +4,7 @@ #include "qrhi_p.h" #include #include +#include "private/qloggingregistry_p.h" #include "qrhinull_p.h" #ifndef QT_NO_OPENGL @@ -24,8 +25,12 @@ QT_BEGIN_NAMESPACE -Q_LOGGING_CATEGORY(QRHI_LOG_INFO, "qt.rhi.general") -Q_STATIC_LOGGING_CATEGORY(QRHI_LOG_RUB, "qt.rhi.rub") +// Play nice with QSG_INFO since that is still the most commonly used +// way to get graphics info printed from Qt Quick apps, and the Quick +// scenegraph is our primary user. +Q_LOGGING_CATEGORY_WITH_ENV_OVERRIDE(QRHI_LOG_INFO, "QSG_INFO", "qt.rhi.general") + +Q_LOGGING_CATEGORY(QRHI_LOG_RUB, "qt.rhi.rub") /*! \class QRhi @@ -8779,22 +8784,12 @@ QRhi::~QRhi() delete d; } -bool QRhiImplementation::rubLogEnabled = false; - void QRhiImplementation::prepareForCreate(QRhi *rhi, QRhi::Implementation impl, QRhi::Flags flags) { q = rhi; - // Play nice with QSG_INFO since that is still the most commonly used - // way to get graphics info printed from Qt Quick apps, and the Quick - // scenegraph is our primary user. - if (qEnvironmentVariableIsSet("QSG_INFO")) - const_cast(QRHI_LOG_INFO()).setEnabled(QtDebugMsg, true); - debugMarkers = flags.testFlag(QRhi::EnableDebugMarkers); - rubLogEnabled = QRHI_LOG_RUB().isDebugEnabled(); - implType = impl; implThread = QThread::currentThread(); } @@ -9667,7 +9662,7 @@ void QRhiResourceUpdateBatchPrivate::free() bufferLargeAllocTotal += op.data.largeAlloc(); // alloc when > 1 KB } - if (rhi->rubLogEnabled) { + if (QRHI_LOG_RUB().isDebugEnabled()) { qDebug() << "[rub] release to pool upd.batch #" << poolIndex << "/ bufferOps active" << activeBufferOpCount << "of" << bufferOps.count() @@ -11185,8 +11180,7 @@ QRhi::FrameOpResult QRhi::beginFrame(QRhiSwapChain *swapChain, BeginFrameFlags f if (d->inFrame) qWarning("Attempted to call beginFrame() within a still active frame; ignored"); - if (d->rubLogEnabled) - qDebug("[rub] new frame"); + qCDebug(QRHI_LOG_RUB) << "[rub] new frame"; QRhi::FrameOpResult r = !d->inFrame ? d->beginFrame(swapChain, flags) : FrameOpSuccess; if (r == FrameOpSuccess) @@ -11336,8 +11330,7 @@ QRhi::FrameOpResult QRhi::beginOffscreenFrame(QRhiCommandBuffer **cb, BeginFrame if (d->inFrame) qWarning("Attempted to call beginOffscreenFrame() within a still active frame; ignored"); - if (d->rubLogEnabled) - qDebug("[rub] new offscreen frame"); + qCDebug(QRHI_LOG_RUB) << "[rub] new offscreen frame"; QRhi::FrameOpResult r = !d->inFrame ? d->beginOffscreenFrame(cb, flags) : FrameOpSuccess; if (r == FrameOpSuccess) diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h index a1b159fa745..590682a52f8 100644 --- a/src/gui/rhi/qrhi_p.h +++ b/src/gui/rhi/qrhi_p.h @@ -29,6 +29,7 @@ QT_BEGIN_NAMESPACE #define QRHI_RES_RHI(t) t *rhiD = static_cast(m_rhi) Q_DECLARE_LOGGING_CATEGORY(QRHI_LOG_INFO) +Q_DECLARE_LOGGING_CATEGORY(QRHI_LOG_RUB) class QRhiImplementation { @@ -259,7 +260,6 @@ private: QHash keyedCleanupCallbacks; QElapsedTimer pipelineCreationTimer; qint64 accumulatedPipelineCreationTime = 0; - static bool rubLogEnabled; friend class QRhi; friend class QRhiResourceUpdateBatchPrivate; @@ -374,7 +374,7 @@ public: if (!d) { d = new QRhiBufferDataPrivate; } else if (d->ref != 1) { - if (QRhiImplementation::rubLogEnabled) + if (QRHI_LOG_RUB().isDebugEnabled()) qDebug("[rub] QRhiBufferData %p/%p new backing due to no-copy detach, ref was %d", this, d, d->ref); d->ref -= 1; d = new QRhiBufferDataPrivate; @@ -384,7 +384,7 @@ public: memcpy(d->data, s, size); } else { if (d->largeAlloc < size) { - if (QRhiImplementation::rubLogEnabled) + if (QRHI_LOG_RUB().isDebugEnabled()) qDebug("[rub] QRhiBufferData %p/%p new large data allocation %u -> %u", this, d, d->largeAlloc, size); delete[] d->largeData; d->largeAlloc = size;