From 253f34082f526ff1ffd9eaefac73cc9aa616ab2a Mon Sep 17 00:00:00 2001 From: David Faure Date: Wed, 9 Apr 2025 14:24:04 +0200 Subject: [PATCH] QPointer: fix data race on ExternalRefCountData The creation of the ExternalRefCountData was published with testAndSetOrdered() but the loadRelaxed() would only load the pointer value, not the effects of the constructor. Pick-to: 6.9 6.8 6.5 Fixes: QTBUG-135640 Change-Id: I3acbc51e763e8a291be3f7036e0d9cd3965a2ce8 Reviewed-by: Thiago Macieira Reviewed-by: Giuseppe D'Angelo --- src/corelib/tools/qsharedpointer.cpp | 2 +- .../corelib/kernel/qpointer/tst_qpointer.cpp | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/corelib/tools/qsharedpointer.cpp b/src/corelib/tools/qsharedpointer.cpp index ae002b1c141..5899cd9a548 100644 --- a/src/corelib/tools/qsharedpointer.cpp +++ b/src/corelib/tools/qsharedpointer.cpp @@ -1531,7 +1531,7 @@ QtSharedPointer::ExternalRefCountData *QtSharedPointer::ExternalRefCountData::ge QObjectPrivate *d = QObjectPrivate::get(const_cast(obj)); Q_ASSERT_X(!d->wasDeleted, "QWeakPointer", "Detected QWeakPointer creation in a QObject being deleted"); - ExternalRefCountData *that = d->sharedRefcount.loadRelaxed(); + ExternalRefCountData *that = d->sharedRefcount.loadAcquire(); if (that) { that->weakref.ref(); return that; diff --git a/tests/auto/corelib/kernel/qpointer/tst_qpointer.cpp b/tests/auto/corelib/kernel/qpointer/tst_qpointer.cpp index fa95fe85f7b..7968aa6fcb4 100644 --- a/tests/auto/corelib/kernel/qpointer/tst_qpointer.cpp +++ b/tests/auto/corelib/kernel/qpointer/tst_qpointer.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -34,6 +35,7 @@ private slots: void disconnect(); void castDuringDestruction(); void threadSafety(); + void raceCondition(); void qvariantCast(); void constPointer(); @@ -529,6 +531,41 @@ void tst_QPointer::threadSafety() owner.wait(); } +void tst_QPointer::raceCondition() +{ + const int NUM_THREADS = 20; + const int ITERATIONS_PER_THREAD = 10; + + QSemaphore startSemaphore; + + QObject targetObject; + + std::vector> threads; + threads.reserve(NUM_THREADS); + + for (int i = 0; i < NUM_THREADS; ++i) { + QThread *thread = + QThread::create([&startSemaphore, &targetObject, ITERATIONS_PER_THREAD]() { + startSemaphore.acquire(); + + for (int j = 0; j < ITERATIONS_PER_THREAD; ++j) { + QPointer pointer(&targetObject); + Q_UNUSED(pointer); + } + }); + + threads.emplace_back(thread); + thread->start(); + } + + QTest::qWait(100); + startSemaphore.release(NUM_THREADS); + + for (const auto &thread : threads) { + QVERIFY(thread->wait(30000)); + } +} + void tst_QPointer::qvariantCast() { QFile file;