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 <laszlo.agocs@qt.io>
(cherry picked from commit 3cc947e98f6237c86160589143e69c13c965e226)
This commit is contained in:
David Faure 2025-03-09 14:02:05 +01:00
parent e0b17563b8
commit b3f39fdd55
2 changed files with 13 additions and 20 deletions

View File

@ -4,6 +4,7 @@
#include "qrhi_p.h" #include "qrhi_p.h"
#include <qmath.h> #include <qmath.h>
#include <QLoggingCategory> #include <QLoggingCategory>
#include "private/qloggingregistry_p.h"
#include "qrhinull_p.h" #include "qrhinull_p.h"
#ifndef QT_NO_OPENGL #ifndef QT_NO_OPENGL
@ -24,8 +25,12 @@
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
Q_LOGGING_CATEGORY(QRHI_LOG_INFO, "qt.rhi.general") // Play nice with QSG_INFO since that is still the most commonly used
Q_STATIC_LOGGING_CATEGORY(QRHI_LOG_RUB, "qt.rhi.rub") // 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 \class QRhi
@ -8779,22 +8784,12 @@ QRhi::~QRhi()
delete d; delete d;
} }
bool QRhiImplementation::rubLogEnabled = false;
void QRhiImplementation::prepareForCreate(QRhi *rhi, QRhi::Implementation impl, QRhi::Flags flags) void QRhiImplementation::prepareForCreate(QRhi *rhi, QRhi::Implementation impl, QRhi::Flags flags)
{ {
q = rhi; 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<QLoggingCategory &>(QRHI_LOG_INFO()).setEnabled(QtDebugMsg, true);
debugMarkers = flags.testFlag(QRhi::EnableDebugMarkers); debugMarkers = flags.testFlag(QRhi::EnableDebugMarkers);
rubLogEnabled = QRHI_LOG_RUB().isDebugEnabled();
implType = impl; implType = impl;
implThread = QThread::currentThread(); implThread = QThread::currentThread();
} }
@ -9667,7 +9662,7 @@ void QRhiResourceUpdateBatchPrivate::free()
bufferLargeAllocTotal += op.data.largeAlloc(); // alloc when > 1 KB bufferLargeAllocTotal += op.data.largeAlloc(); // alloc when > 1 KB
} }
if (rhi->rubLogEnabled) { if (QRHI_LOG_RUB().isDebugEnabled()) {
qDebug() << "[rub] release to pool upd.batch #" << poolIndex qDebug() << "[rub] release to pool upd.batch #" << poolIndex
<< "/ bufferOps active" << activeBufferOpCount << "/ bufferOps active" << activeBufferOpCount
<< "of" << bufferOps.count() << "of" << bufferOps.count()
@ -11185,8 +11180,7 @@ QRhi::FrameOpResult QRhi::beginFrame(QRhiSwapChain *swapChain, BeginFrameFlags f
if (d->inFrame) if (d->inFrame)
qWarning("Attempted to call beginFrame() within a still active frame; ignored"); qWarning("Attempted to call beginFrame() within a still active frame; ignored");
if (d->rubLogEnabled) qCDebug(QRHI_LOG_RUB) << "[rub] new frame";
qDebug("[rub] new frame");
QRhi::FrameOpResult r = !d->inFrame ? d->beginFrame(swapChain, flags) : FrameOpSuccess; QRhi::FrameOpResult r = !d->inFrame ? d->beginFrame(swapChain, flags) : FrameOpSuccess;
if (r == FrameOpSuccess) if (r == FrameOpSuccess)
@ -11336,8 +11330,7 @@ QRhi::FrameOpResult QRhi::beginOffscreenFrame(QRhiCommandBuffer **cb, BeginFrame
if (d->inFrame) if (d->inFrame)
qWarning("Attempted to call beginOffscreenFrame() within a still active frame; ignored"); qWarning("Attempted to call beginOffscreenFrame() within a still active frame; ignored");
if (d->rubLogEnabled) qCDebug(QRHI_LOG_RUB) << "[rub] new offscreen frame";
qDebug("[rub] new offscreen frame");
QRhi::FrameOpResult r = !d->inFrame ? d->beginOffscreenFrame(cb, flags) : FrameOpSuccess; QRhi::FrameOpResult r = !d->inFrame ? d->beginOffscreenFrame(cb, flags) : FrameOpSuccess;
if (r == FrameOpSuccess) if (r == FrameOpSuccess)

View File

@ -29,6 +29,7 @@ QT_BEGIN_NAMESPACE
#define QRHI_RES_RHI(t) t *rhiD = static_cast<t *>(m_rhi) #define QRHI_RES_RHI(t) t *rhiD = static_cast<t *>(m_rhi)
Q_DECLARE_LOGGING_CATEGORY(QRHI_LOG_INFO) Q_DECLARE_LOGGING_CATEGORY(QRHI_LOG_INFO)
Q_DECLARE_LOGGING_CATEGORY(QRHI_LOG_RUB)
class QRhiImplementation class QRhiImplementation
{ {
@ -259,7 +260,6 @@ private:
QHash<const void *, QRhi::CleanupCallback> keyedCleanupCallbacks; QHash<const void *, QRhi::CleanupCallback> keyedCleanupCallbacks;
QElapsedTimer pipelineCreationTimer; QElapsedTimer pipelineCreationTimer;
qint64 accumulatedPipelineCreationTime = 0; qint64 accumulatedPipelineCreationTime = 0;
static bool rubLogEnabled;
friend class QRhi; friend class QRhi;
friend class QRhiResourceUpdateBatchPrivate; friend class QRhiResourceUpdateBatchPrivate;
@ -374,7 +374,7 @@ public:
if (!d) { if (!d) {
d = new QRhiBufferDataPrivate; d = new QRhiBufferDataPrivate;
} else if (d->ref != 1) { } 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); qDebug("[rub] QRhiBufferData %p/%p new backing due to no-copy detach, ref was %d", this, d, d->ref);
d->ref -= 1; d->ref -= 1;
d = new QRhiBufferDataPrivate; d = new QRhiBufferDataPrivate;
@ -384,7 +384,7 @@ public:
memcpy(d->data, s, size); memcpy(d->data, s, size);
} else { } else {
if (d->largeAlloc < size) { 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); qDebug("[rub] QRhiBufferData %p/%p new large data allocation %u -> %u", this, d, d->largeAlloc, size);
delete[] d->largeData; delete[] d->largeData;
d->largeAlloc = size; d->largeAlloc = size;