summaryrefslogtreecommitdiffstats
path: root/src/corelib
diff options
context:
space:
mode:
authorGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2019-10-11 00:42:08 +0200
committerGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2019-11-13 16:22:40 +0100
commit782df5b41dd3ab098fd1d3233339079487e1812f (patch)
treea2727ed7dda4ecff4331e99d05f085bb7fc01184 /src/corelib
parent4e0d5498eb7ba401e6697182ce74b34d439ecf76 (diff)
Make QObjectPrivate::threadData a proper atomic
QObjectPrivate::threadData used to be a QThreadData *, and was read and written from multiple threads without proper synchronization. As an example, it was read from QCoreApplication::postEvent and written from QObject::moveToThread, therefore causing UB. Port threadData to a proper atomic, removing the races. Fix all usage points. In general, QObject is documented to be simply reentrant, not thread-safe, and certain bits (e.g. timers, moveToThread) are not even reentrant. The reasoning therefore is that a given QObject's threadData is not supposed to be touched by multiple threads without some synchronization happening elsewhere, and therefore relaxed loads should be sufficient. As drive-by change: refactor QCoreApplication::postEvent. It was particularly subtle, because it had a loop using a volatile to cope with the possibility of the receiver object switching thread while we tried to lock its thread's event queue. However, volatile does not achieve any synchronization, so drop it, and refactor the algorithm using better locking primitives. Put this algorithm in a common place, and also reuse it from removePostedEvents, which was lacking any synchronization. Change-Id: Icc755f7eb418ff54b33db4bdd87fd8eaf4e82c7a Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src/corelib')
-rw-r--r--src/corelib/io/qprocess_unix.cpp6
-rw-r--r--src/corelib/io/qprocess_win.cpp2
-rw-r--r--src/corelib/kernel/qcoreapplication.cpp112
-rw-r--r--src/corelib/kernel/qcoreapplication_p.h10
-rw-r--r--src/corelib/kernel/qcoreapplication_win.cpp2
-rw-r--r--src/corelib/kernel/qeventdispatcher_unix.cpp6
-rw-r--r--src/corelib/kernel/qeventdispatcher_win.cpp2
-rw-r--r--src/corelib/kernel/qeventloop.cpp43
-rw-r--r--src/corelib/kernel/qobject.cpp52
-rw-r--r--src/corelib/kernel/qobject_p.h9
-rw-r--r--src/corelib/kernel/qsocketnotifier.cpp15
-rw-r--r--src/corelib/kernel/qwineventnotifier.cpp6
12 files changed, 152 insertions, 113 deletions
diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 0c80daa024..9edb4a6d11 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -246,7 +246,7 @@ bool QProcessPrivate::openChannel(Channel &channel)
return false;
// create the socket notifiers
- if (threadData->hasEventDispatcher()) {
+ if (threadData.loadRelaxed()->hasEventDispatcher()) {
if (&channel == &stdinChannel) {
channel.notifier = new QSocketNotifier(channel.pipe[1],
QSocketNotifier::Write, q);
@@ -377,7 +377,7 @@ void QProcessPrivate::startProcess()
return;
}
- if (threadData->hasEventDispatcher()) {
+ if (threadData.loadRelaxed()->hasEventDispatcher()) {
startupSocketNotifier = new QSocketNotifier(childStartedPipe[0],
QSocketNotifier::Read, q);
QObject::connect(startupSocketNotifier, SIGNAL(activated(int)),
@@ -517,7 +517,7 @@ void QProcessPrivate::startProcess()
if (stderrChannel.pipe[0] != -1)
::fcntl(stderrChannel.pipe[0], F_SETFL, ::fcntl(stderrChannel.pipe[0], F_GETFL) | O_NONBLOCK);
- if (threadData->eventDispatcher.loadAcquire()) {
+ if (threadData.loadRelaxed()->eventDispatcher.loadAcquire()) {
deathNotifier = new QSocketNotifier(forkfd, QSocketNotifier::Read, q);
QObject::connect(deathNotifier, SIGNAL(activated(int)),
q, SLOT(_q_processDied()));
diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp
index 3ba86063e3..05af5a5aee 100644
--- a/src/corelib/io/qprocess_win.cpp
+++ b/src/corelib/io/qprocess_win.cpp
@@ -590,7 +590,7 @@ void QProcessPrivate::startProcess()
if (!pid)
return;
- if (threadData->hasEventDispatcher()) {
+ if (threadData.loadRelaxed()->hasEventDispatcher()) {
processFinishedNotifier = new QWinEventNotifier(pid->hProcess, q);
QObject::connect(processFinishedNotifier, SIGNAL(activated(HANDLE)), q, SLOT(_q_processDied()));
processFinishedNotifier->setEnabled(true);
diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp
index e25049f821..6d95979f76 100644
--- a/src/corelib/kernel/qcoreapplication.cpp
+++ b/src/corelib/kernel/qcoreapplication.cpp
@@ -135,23 +135,6 @@
QT_BEGIN_NAMESPACE
-#ifndef QT_NO_QOBJECT
-class QMutexUnlocker
-{
-public:
- inline explicit QMutexUnlocker(QMutex *m)
- : mtx(m)
- { }
- inline ~QMutexUnlocker() { unlock(); }
- inline void unlock() { if (mtx) mtx->unlock(); mtx = 0; }
-
-private:
- Q_DISABLE_COPY(QMutexUnlocker)
-
- QMutex *mtx;
-};
-#endif
-
#if defined(Q_OS_WIN) || defined(Q_OS_MAC)
extern QString qAppFileName();
#endif
@@ -517,25 +500,27 @@ QCoreApplicationPrivate::~QCoreApplicationPrivate()
void QCoreApplicationPrivate::cleanupThreadData()
{
- if (threadData && !threadData_clean) {
+ auto thisThreadData = threadData.loadRelaxed();
+
+ if (thisThreadData && !threadData_clean) {
#if QT_CONFIG(thread)
- void *data = &threadData->tls;
+ void *data = &thisThreadData->tls;
QThreadStorageData::finish((void **)data);
#endif
// need to clear the state of the mainData, just in case a new QCoreApplication comes along.
- const auto locker = qt_scoped_lock(threadData->postEventList.mutex);
- for (int i = 0; i < threadData->postEventList.size(); ++i) {
- const QPostEvent &pe = threadData->postEventList.at(i);
+ const auto locker = qt_scoped_lock(thisThreadData->postEventList.mutex);
+ for (int i = 0; i < thisThreadData->postEventList.size(); ++i) {
+ const QPostEvent &pe = thisThreadData->postEventList.at(i);
if (pe.event) {
--pe.receiver->d_func()->postedEvents;
pe.event->posted = false;
delete pe.event;
}
}
- threadData->postEventList.clear();
- threadData->postEventList.recursion = 0;
- threadData->quitNow = false;
+ thisThreadData->postEventList.clear();
+ thisThreadData->postEventList.recursion = 0;
+ thisThreadData->quitNow = false;
threadData_clean = true;
}
}
@@ -858,7 +843,8 @@ void QCoreApplicationPrivate::init()
#ifndef QT_NO_QOBJECT
// use the event dispatcher created by the app programmer (if any)
Q_ASSERT(!eventDispatcher);
- eventDispatcher = threadData->eventDispatcher.loadRelaxed();
+ auto thisThreadData = threadData.loadRelaxed();
+ eventDispatcher = thisThreadData->eventDispatcher.loadRelaxed();
// otherwise we create one
if (!eventDispatcher)
@@ -866,11 +852,11 @@ void QCoreApplicationPrivate::init()
Q_ASSERT(eventDispatcher);
if (!eventDispatcher->parent()) {
- eventDispatcher->moveToThread(threadData->thread.loadAcquire());
+ eventDispatcher->moveToThread(thisThreadData->thread.loadAcquire());
eventDispatcher->setParent(q);
}
- threadData->eventDispatcher = eventDispatcher;
+ thisThreadData->eventDispatcher = eventDispatcher;
eventDispatcherReady();
#endif
@@ -914,7 +900,7 @@ QCoreApplication::~QCoreApplication()
#endif
#ifndef QT_NO_QOBJECT
- d_func()->threadData->eventDispatcher = nullptr;
+ d_func()->threadData.loadRelaxed()->eventDispatcher = nullptr;
if (QCoreApplicationPrivate::eventDispatcher)
QCoreApplicationPrivate::eventDispatcher->closingDown();
QCoreApplicationPrivate::eventDispatcher = nullptr;
@@ -1185,7 +1171,7 @@ static bool doNotify(QObject *receiver, QEvent *event)
bool QCoreApplicationPrivate::sendThroughApplicationEventFilters(QObject *receiver, QEvent *event)
{
// We can't access the application event filters outside of the main thread (race conditions)
- Q_ASSERT(receiver->d_func()->threadData->thread.loadAcquire() == mainThread());
+ Q_ASSERT(receiver->d_func()->threadData.loadRelaxed()->thread.loadAcquire() == mainThread());
if (extraData) {
// application event filters are only called for objects in the GUI thread
@@ -1238,7 +1224,7 @@ bool QCoreApplicationPrivate::notify_helper(QObject *receiver, QEvent * event)
// send to all application event filters (only does anything in the main thread)
if (QCoreApplication::self
- && receiver->d_func()->threadData->thread.loadAcquire() == mainThread()
+ && receiver->d_func()->threadData.loadRelaxed()->thread.loadAcquire() == mainThread()
&& QCoreApplication::self->d_func()->sendThroughApplicationEventFilters(receiver, event)) {
filtered = true;
return filtered;
@@ -1414,7 +1400,7 @@ int QCoreApplication::exec()
void QCoreApplicationPrivate::execCleanup()
{
- threadData->quitNow = false;
+ threadData.loadRelaxed()->quitNow = false;
in_exec = false;
if (!aboutToQuitEmitted)
emit q_func()->aboutToQuit(QCoreApplication::QPrivateSignal());
@@ -1451,7 +1437,7 @@ void QCoreApplication::exit(int returnCode)
{
if (!self)
return;
- QThreadData *data = self->d_func()->threadData;
+ QThreadData *data = self->d_func()->threadData.loadRelaxed();
data->quitNow = true;
for (int i = 0; i < data->eventLoops.size(); ++i) {
QEventLoop *eventLoop = data->eventLoops.at(i);
@@ -1501,6 +1487,38 @@ bool QCoreApplication::sendSpontaneousEvent(QObject *receiver, QEvent *event)
#endif // QT_NO_QOBJECT
+QCoreApplicationPrivate::QPostEventListLocker QCoreApplicationPrivate::lockThreadPostEventList(QObject *object)
+{
+ QPostEventListLocker locker;
+
+ if (!object) {
+ locker.threadData = QThreadData::current();
+ locker.locker = qt_unique_lock(locker.threadData->postEventList.mutex);
+ return locker;
+ }
+
+ auto &threadData = QObjectPrivate::get(object)->threadData;
+
+ // if object has moved to another thread, follow it
+ for (;;) {
+ // synchronizes with the storeRelease in QObject::moveToThread
+ locker.threadData = threadData.loadAcquire();
+ if (!locker.threadData) {
+ // destruction in progress
+ return locker;
+ }
+
+ auto temporaryLocker = qt_unique_lock(locker.threadData->postEventList.mutex);
+ if (locker.threadData == threadData.loadAcquire()) {
+ locker.locker = std::move(temporaryLocker);
+ break;
+ }
+ }
+
+ Q_ASSERT(locker.threadData);
+ return locker;
+}
+
/*!
\since 4.3
@@ -1536,32 +1554,14 @@ void QCoreApplication::postEvent(QObject *receiver, QEvent *event, int priority)
return;
}
- QThreadData * volatile * pdata = &receiver->d_func()->threadData;
- QThreadData *data = *pdata;
- if (!data) {
+ auto locker = QCoreApplicationPrivate::lockThreadPostEventList(receiver);
+ if (!locker.threadData) {
// posting during destruction? just delete the event to prevent a leak
delete event;
return;
}
- // lock the post event mutex
- data->postEventList.mutex.lock();
-
- // if object has moved to another thread, follow it
- while (data != *pdata) {
- data->postEventList.mutex.unlock();
-
- data = *pdata;
- if (!data) {
- // posting during destruction? just delete the event to prevent a leak
- delete event;
- return;
- }
-
- data->postEventList.mutex.lock();
- }
-
- QMutexUnlocker locker(&data->postEventList.mutex);
+ QThreadData *data = locker.threadData;
// if this is one of the compressible events, do compression
if (receiver->d_func()->postedEvents
@@ -1860,8 +1860,8 @@ void QCoreApplicationPrivate::sendPostedEvents(QObject *receiver, int event_type
void QCoreApplication::removePostedEvents(QObject *receiver, int eventType)
{
- QThreadData *data = receiver ? receiver->d_func()->threadData : QThreadData::current();
- auto locker = qt_unique_lock(data->postEventList.mutex);
+ auto locker = QCoreApplicationPrivate::lockThreadPostEventList(receiver);
+ QThreadData *data = locker.threadData;
// the QObject destructor calls this function directly. this can
// happen while the event loop is in the middle of posting events,
diff --git a/src/corelib/kernel/qcoreapplication_p.h b/src/corelib/kernel/qcoreapplication_p.h
index 3bad42d076..9d2fde619c 100644
--- a/src/corelib/kernel/qcoreapplication_p.h
+++ b/src/corelib/kernel/qcoreapplication_p.h
@@ -61,6 +61,7 @@
#endif
#ifndef QT_NO_QOBJECT
#include "private/qobject_p.h"
+#include "private/qlocking_p.h"
#endif
#ifdef Q_OS_MACOS
@@ -140,6 +141,15 @@ public:
static void checkReceiverThread(QObject *receiver);
void cleanupThreadData();
+
+ struct QPostEventListLocker
+ {
+ QThreadData *threadData;
+ std::unique_lock<QMutex> locker;
+
+ void unlock() { locker.unlock(); }
+ };
+ static QPostEventListLocker lockThreadPostEventList(QObject *object);
#endif // QT_NO_QOBJECT
int &argc;
diff --git a/src/corelib/kernel/qcoreapplication_win.cpp b/src/corelib/kernel/qcoreapplication_win.cpp
index 961b96710e..765f129758 100644
--- a/src/corelib/kernel/qcoreapplication_win.cpp
+++ b/src/corelib/kernel/qcoreapplication_win.cpp
@@ -918,7 +918,7 @@ QDebug operator<<(QDebug dbg, const MSG &msg)
#ifndef QT_NO_QOBJECT
void QCoreApplicationPrivate::removePostedTimerEvent(QObject *object, int timerId)
{
- QThreadData *data = object->d_func()->threadData;
+ QThreadData *data = object->d_func()->threadData.loadRelaxed();
const auto locker = qt_scoped_lock(data->postEventList.mutex);
if (data->postEventList.size() == 0)
diff --git a/src/corelib/kernel/qeventdispatcher_unix.cpp b/src/corelib/kernel/qeventdispatcher_unix.cpp
index 5bc65b7110..2492d70005 100644
--- a/src/corelib/kernel/qeventdispatcher_unix.cpp
+++ b/src/corelib/kernel/qeventdispatcher_unix.cpp
@@ -463,13 +463,15 @@ bool QEventDispatcherUNIX::processEvents(QEventLoop::ProcessEventsFlags flags)
// we are awake, broadcast it
emit awake();
- QCoreApplicationPrivate::sendPostedEvents(0, 0, d->threadData);
+
+ auto threadData = d->threadData.loadRelaxed();
+ QCoreApplicationPrivate::sendPostedEvents(0, 0, threadData);
const bool include_timers = (flags & QEventLoop::X11ExcludeTimers) == 0;
const bool include_notifiers = (flags & QEventLoop::ExcludeSocketNotifiers) == 0;
const bool wait_for_events = flags & QEventLoop::WaitForMoreEvents;
- const bool canWait = (d->threadData->canWaitLocked()
+ const bool canWait = (threadData->canWaitLocked()
&& !d->interrupt.loadRelaxed()
&& wait_for_events);
diff --git a/src/corelib/kernel/qeventdispatcher_win.cpp b/src/corelib/kernel/qeventdispatcher_win.cpp
index 517ba17fb2..8616631603 100644
--- a/src/corelib/kernel/qeventdispatcher_win.cpp
+++ b/src/corelib/kernel/qeventdispatcher_win.cpp
@@ -1048,7 +1048,7 @@ bool QEventDispatcherWin32::event(QEvent *e)
void QEventDispatcherWin32::sendPostedEvents()
{
Q_D(QEventDispatcherWin32);
- QCoreApplicationPrivate::sendPostedEvents(0, 0, d->threadData);
+ QCoreApplicationPrivate::sendPostedEvents(0, 0, d->threadData.loadRelaxed());
}
HWND QEventDispatcherWin32::internalHwnd()
diff --git a/src/corelib/kernel/qeventloop.cpp b/src/corelib/kernel/qeventloop.cpp
index eacd0c4e73..5a5dfb06aa 100644
--- a/src/corelib/kernel/qeventloop.cpp
+++ b/src/corelib/kernel/qeventloop.cpp
@@ -106,7 +106,7 @@ QEventLoop::QEventLoop(QObject *parent)
if (!QCoreApplication::instance() && QCoreApplicationPrivate::threadRequiresCoreApplication()) {
qWarning("QEventLoop: Cannot be used without QApplication");
} else {
- d->threadData->ensureEventDispatcher();
+ d->threadData.loadRelaxed()->ensureEventDispatcher();
}
}
@@ -133,9 +133,10 @@ QEventLoop::~QEventLoop()
bool QEventLoop::processEvents(ProcessEventsFlags flags)
{
Q_D(QEventLoop);
- if (!d->threadData->hasEventDispatcher())
+ auto threadData = d->threadData.loadRelaxed();
+ if (!threadData->hasEventDispatcher())
return false;
- return d->threadData->eventDispatcher.loadRelaxed()->processEvents(flags);
+ return threadData->eventDispatcher.loadRelaxed()->processEvents(flags);
}
/*!
@@ -164,9 +165,11 @@ bool QEventLoop::processEvents(ProcessEventsFlags flags)
int QEventLoop::exec(ProcessEventsFlags flags)
{
Q_D(QEventLoop);
+ auto threadData = d->threadData.loadRelaxed();
+
//we need to protect from race condition with QThread::exit
- QMutexLocker locker(&static_cast<QThreadPrivate *>(QObjectPrivate::get(d->threadData->thread.loadAcquire()))->mutex);
- if (d->threadData->quitNow)
+ QMutexLocker locker(&static_cast<QThreadPrivate *>(QObjectPrivate::get(threadData->thread.loadAcquire()))->mutex);
+ if (threadData->quitNow)
return -1;
if (d->inExec) {
@@ -183,8 +186,11 @@ int QEventLoop::exec(ProcessEventsFlags flags)
{
d->inExec = true;
d->exit.storeRelease(false);
- ++d->threadData->loopLevel;
- d->threadData->eventLoops.push(d->q_func());
+
+ auto threadData = d->threadData.loadRelaxed();
+ ++threadData->loopLevel;
+ threadData->eventLoops.push(d->q_func());
+
locker.unlock();
}
@@ -198,11 +204,12 @@ int QEventLoop::exec(ProcessEventsFlags flags)
"QCoreApplication::notify() and catch all exceptions there.\n");
}
locker.relock();
- QEventLoop *eventLoop = d->threadData->eventLoops.pop();
+ auto threadData = d->threadData.loadRelaxed();
+ QEventLoop *eventLoop = threadData->eventLoops.pop();
Q_ASSERT_X(eventLoop == d->q_func(), "QEventLoop::exec()", "internal error");
Q_UNUSED(eventLoop); // --release warning
d->inExec = false;
- --d->threadData->loopLevel;
+ --threadData->loopLevel;
}
};
LoopReference ref(d, locker);
@@ -217,7 +224,7 @@ int QEventLoop::exec(ProcessEventsFlags flags)
// exception, which returns control to the browser while preserving the C++ stack.
// Event processing then continues as normal. The sleep call below never returns.
// QTBUG-70185
- if (d->threadData->loopLevel > 1)
+ if (threadData->loopLevel > 1)
emscripten_sleep(1);
#endif
@@ -247,7 +254,7 @@ int QEventLoop::exec(ProcessEventsFlags flags)
void QEventLoop::processEvents(ProcessEventsFlags flags, int maxTime)
{
Q_D(QEventLoop);
- if (!d->threadData->hasEventDispatcher())
+ if (!d->threadData.loadRelaxed()->hasEventDispatcher())
return;
QElapsedTimer start;
@@ -276,21 +283,22 @@ void QEventLoop::processEvents(ProcessEventsFlags flags, int maxTime)
void QEventLoop::exit(int returnCode)
{
Q_D(QEventLoop);
- if (!d->threadData->hasEventDispatcher())
+ auto threadData = d->threadData.loadAcquire();
+ if (!threadData->hasEventDispatcher())
return;
d->returnCode.storeRelaxed(returnCode);
d->exit.storeRelease(true);
- d->threadData->eventDispatcher.loadRelaxed()->interrupt();
+ threadData->eventDispatcher.loadRelaxed()->interrupt();
#ifdef Q_OS_WASM
// QEventLoop::exec() never returns in emscripten. We implement approximate behavior here.
// QTBUG-70185
- if (d->threadData->loopLevel == 1) {
+ if (threadData->loopLevel == 1) {
emscripten_force_exit(returnCode);
} else {
d->inExec = false;
- --d->threadData->loopLevel;
+ --threadData->loopLevel;
}
#endif
}
@@ -316,9 +324,10 @@ bool QEventLoop::isRunning() const
void QEventLoop::wakeUp()
{
Q_D(QEventLoop);
- if (!d->threadData->hasEventDispatcher())
+ auto threadData = d->threadData.loadAcquire();
+ if (!threadData->hasEventDispatcher())
return;
- d->threadData->eventDispatcher.loadRelaxed()->wakeUp();
+ threadData->eventDispatcher.loadRelaxed()->wakeUp();
}
diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index bb1b48b0a6..cee885c0fe 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -212,11 +212,12 @@ QObjectPrivate::QObjectPrivate(int version)
QObjectPrivate::~QObjectPrivate()
{
+ auto thisThreadData = threadData.loadRelaxed();
if (extraData && !extraData->runningTimers.isEmpty()) {
- if (Q_LIKELY(threadData->thread.loadAcquire() == QThread::currentThread())) {
+ if (Q_LIKELY(thisThreadData->thread.loadAcquire() == QThread::currentThread())) {
// unregister pending timers
- if (threadData->hasEventDispatcher())
- threadData->eventDispatcher.loadRelaxed()->unregisterTimers(q_ptr);
+ if (thisThreadData->hasEventDispatcher())
+ thisThreadData->eventDispatcher.loadRelaxed()->unregisterTimers(q_ptr);
// release the timer ids back to the pool
for (int i = 0; i < extraData->runningTimers.size(); ++i)
@@ -229,7 +230,7 @@ QObjectPrivate::~QObjectPrivate()
if (postedEvents)
QCoreApplication::removePostedEvents(q_ptr, 0);
- threadData->deref();
+ thisThreadData->deref();
if (metaObject) metaObject->objectDestroyed(q_ptr);
@@ -920,11 +921,12 @@ QObject::QObject(QObjectPrivate &dd, QObject *parent)
Q_D(QObject);
d_ptr->q_ptr = this;
- d->threadData = (parent && !parent->thread()) ? parent->d_func()->threadData : QThreadData::current();
- d->threadData->ref();
+ auto threadData = (parent && !parent->thread()) ? parent->d_func()->threadData.loadRelaxed() : QThreadData::current();
+ threadData->ref();
+ d->threadData.storeRelaxed(threadData);
if (parent) {
QT_TRY {
- if (!check_parent_thread(parent, parent ? parent->d_func()->threadData : 0, d->threadData))
+ if (!check_parent_thread(parent, parent ? parent->d_func()->threadData.loadRelaxed() : 0, threadData))
parent = 0;
if (d->isWidget) {
if (parent) {
@@ -936,7 +938,7 @@ QObject::QObject(QObjectPrivate &dd, QObject *parent)
setParent(parent);
}
} QT_CATCH(...) {
- d->threadData->deref();
+ threadData->deref();
QT_RETHROW;
}
}
@@ -1320,7 +1322,7 @@ bool QObject::event(QEvent *e)
case QEvent::ThreadChange: {
Q_D(QObject);
- QThreadData *threadData = d->threadData;
+ QThreadData *threadData = d->threadData.loadRelaxed();
QAbstractEventDispatcher *eventDispatcher = threadData->eventDispatcher.loadRelaxed();
if (eventDispatcher) {
QList<QAbstractEventDispatcher::TimerInfo> timers = eventDispatcher->registeredTimers(this);
@@ -1487,7 +1489,7 @@ bool QObject::blockSignals(bool block) noexcept
*/
QThread *QObject::thread() const
{
- return d_func()->threadData->thread.loadAcquire();
+ return d_func()->threadData.loadRelaxed()->thread.loadAcquire();
}
/*!
@@ -1534,7 +1536,7 @@ void QObject::moveToThread(QThread *targetThread)
{
Q_D(QObject);
- if (d->threadData->thread.loadAcquire() == targetThread) {
+ if (d->threadData.loadRelaxed()->thread.loadAcquire() == targetThread) {
// object is already in this thread
return;
}
@@ -1550,13 +1552,14 @@ void QObject::moveToThread(QThread *targetThread)
QThreadData *currentData = QThreadData::current();
QThreadData *targetData = targetThread ? QThreadData::get2(targetThread) : nullptr;
- if (d->threadData->thread.loadAcquire() == 0 && currentData == targetData) {
+ QThreadData *thisThreadData = d->threadData.loadRelaxed();
+ if (!thisThreadData->thread.loadAcquire() && currentData == targetData) {
// one exception to the rule: we allow moving objects with no thread affinity to the current thread
currentData = d->threadData;
- } else if (d->threadData != currentData) {
+ } else if (thisThreadData != currentData) {
qWarning("QObject::moveToThread: Current thread (%p) is not the object's thread (%p).\n"
"Cannot move to target thread (%p)\n",
- currentData->thread.loadRelaxed(), d->threadData->thread.loadRelaxed(), targetData ? targetData->thread.loadRelaxed() : nullptr);
+ currentData->thread.loadRelaxed(), thisThreadData->thread.loadRelaxed(), targetData ? targetData->thread.loadRelaxed() : nullptr);
#ifdef Q_OS_MAC
qWarning("You might be loading two sets of Qt binaries into the same process. "
@@ -1653,8 +1656,10 @@ void QObjectPrivate::setThreadData_helper(QThreadData *currentData, QThreadData
// set new thread data
targetData->ref();
- threadData->deref();
- threadData = targetData;
+ threadData.loadRelaxed()->deref();
+
+ // synchronizes with loadAcquire e.g. in QCoreApplication::postEvent
+ threadData.storeRelease(targetData);
for (int i = 0; i < children.size(); ++i) {
QObject *child = children.at(i);
@@ -1666,7 +1671,7 @@ void QObjectPrivate::_q_reregisterTimers(void *pointer)
{
Q_Q(QObject);
QList<QAbstractEventDispatcher::TimerInfo> *timerList = reinterpret_cast<QList<QAbstractEventDispatcher::TimerInfo> *>(pointer);
- QAbstractEventDispatcher *eventDispatcher = threadData->eventDispatcher.loadRelaxed();
+ QAbstractEventDispatcher *eventDispatcher = threadData.loadRelaxed()->eventDispatcher.loadRelaxed();
for (int i = 0; i < timerList->size(); ++i) {
const QAbstractEventDispatcher::TimerInfo &ti = timerList->at(i);
eventDispatcher->registerTimer(ti.timerId, ti.interval, ti.timerType, q);
@@ -1724,7 +1729,9 @@ int QObject::startTimer(int interval, Qt::TimerType timerType)
qWarning("QObject::startTimer: Timers cannot have negative intervals");
return 0;
}
- if (Q_UNLIKELY(!d->threadData->hasEventDispatcher())) {
+
+ auto thisThreadData = d->threadData.loadRelaxed();
+ if (Q_UNLIKELY(!thisThreadData->hasEventDispatcher())) {
qWarning("QObject::startTimer: Timers can only be used with threads started with QThread");
return 0;
}
@@ -1732,7 +1739,7 @@ int QObject::startTimer(int interval, Qt::TimerType timerType)
qWarning("QObject::startTimer: Timers cannot be started from another thread");
return 0;
}
- int timerId = d->threadData->eventDispatcher.loadRelaxed()->registerTimer(interval, timerType, this);
+ int timerId = thisThreadData->eventDispatcher.loadRelaxed()->registerTimer(interval, timerType, this);
if (!d->extraData)
d->extraData = new QObjectPrivate::ExtraData;
d->extraData->runningTimers.append(timerId);
@@ -1806,8 +1813,9 @@ void QObject::killTimer(int id)
return;
}
- if (d->threadData->hasEventDispatcher())
- d->threadData->eventDispatcher.loadRelaxed()->unregisterTimer(id);
+ auto thisThreadData = d->threadData.loadRelaxed();
+ if (thisThreadData->hasEventDispatcher())
+ thisThreadData->eventDispatcher.loadRelaxed()->unregisterTimer(id);
d->extraData->runningTimers.remove(at);
QAbstractEventDispatcherPrivate::releaseTimerId(id);
@@ -3774,7 +3782,7 @@ void doActivate(QObject *sender, int signal_index, void **argv)
list = &signalVector->at(-1);
Qt::HANDLE currentThreadId = QThread::currentThreadId();
- bool inSenderThread = currentThreadId == QObjectPrivate::get(sender)->threadData->threadId.loadRelaxed();
+ bool inSenderThread = currentThreadId == QObjectPrivate::get(sender)->threadData.loadRelaxed()->threadId.loadRelaxed();
// We need to check against the highest connection id to ensure that signals added
// during the signal emission are not emitted in this emission.
diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h
index 45fc27917d..1ebf8e7a07 100644
--- a/src/corelib/kernel/qobject_p.h
+++ b/src/corelib/kernel/qobject_p.h
@@ -374,8 +374,13 @@ public:
}
public:
ExtraData *extraData; // extra data set by the user
- QThreadData *getThreadData() const { return threadData; }
- QThreadData *threadData; // id of the thread that owns the object
+ QThreadData *getThreadData() const { return threadData.loadAcquire(); }
+ // This atomic requires acquire/release semantics in a few places,
+ // e.g. QObject::moveToThread must synchronize with QCoreApplication::postEvent,
+ // because postEvent is thread-safe.
+ // However, most of the code paths involving QObject are only reentrant and
+ // not thread-safe, so synchronization should not be necessary there.
+ QAtomicPointer<QThreadData> threadData; // id of the thread that owns the object
using ConnectionDataPointer = QExplicitlySharedDataPointer<ConnectionData>;
QAtomicPointer<ConnectionData> connections;
diff --git a/src/corelib/kernel/qsocketnotifier.cpp b/src/corelib/kernel/qsocketnotifier.cpp
index 2a246b1204..78269ee605 100644
--- a/src/corelib/kernel/qsocketnotifier.cpp
+++ b/src/corelib/kernel/qsocketnotifier.cpp
@@ -147,12 +147,14 @@ QSocketNotifier::QSocketNotifier(qintptr socket, Type type, QObject *parent)
d->sntype = type;
d->snenabled = true;
+ auto thisThreadData = d->threadData.loadRelaxed();
+
if (socket < 0)
qWarning("QSocketNotifier: Invalid socket specified");
- else if (!d->threadData->hasEventDispatcher())
+ else if (!thisThreadData->hasEventDispatcher())
qWarning("QSocketNotifier: Can only be used with threads started with QThread");
else
- d->threadData->eventDispatcher.loadRelaxed()->registerSocketNotifier(this);
+ thisThreadData->eventDispatcher.loadRelaxed()->registerSocketNotifier(this);
}
/*!
@@ -234,16 +236,19 @@ void QSocketNotifier::setEnabled(bool enable)
return;
d->snenabled = enable;
- if (!d->threadData->hasEventDispatcher()) // perhaps application/thread is shutting down
+
+ auto thisThreadData = d->threadData.loadRelaxed();
+
+ if (!thisThreadData->hasEventDispatcher()) // perhaps application/thread is shutting down
return;
if (Q_UNLIKELY(thread() != QThread::currentThread())) {
qWarning("QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread");
return;
}
if (d->snenabled)
- d->threadData->eventDispatcher.loadRelaxed()->registerSocketNotifier(this);
+ thisThreadData->eventDispatcher.loadRelaxed()->registerSocketNotifier(this);
else
- d->threadData->eventDispatcher.loadRelaxed()->unregisterSocketNotifier(this);
+ thisThreadData->eventDispatcher.loadRelaxed()->unregisterSocketNotifier(this);
}
diff --git a/src/corelib/kernel/qwineventnotifier.cpp b/src/corelib/kernel/qwineventnotifier.cpp
index d2ae9668fe..db5d44b276 100644
--- a/src/corelib/kernel/qwineventnotifier.cpp
+++ b/src/corelib/kernel/qwineventnotifier.cpp
@@ -124,7 +124,7 @@ QWinEventNotifier::QWinEventNotifier(HANDLE hEvent, QObject *parent)
: QObject(*new QWinEventNotifierPrivate(hEvent, false), parent)
{
Q_D(QWinEventNotifier);
- QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher.loadRelaxed();
+ QAbstractEventDispatcher *eventDispatcher = d->threadData.loadRelaxed()->eventDispatcher.loadRelaxed();
if (Q_UNLIKELY(!eventDispatcher)) {
qWarning("QWinEventNotifier: Can only be used with threads started with QThread");
return;
@@ -197,7 +197,7 @@ void QWinEventNotifier::setEnabled(bool enable)
return;
d->enabled = enable;
- QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher.loadRelaxed();
+ QAbstractEventDispatcher *eventDispatcher = d->threadData.loadRelaxed()->eventDispatcher.loadRelaxed();
if (!eventDispatcher) { // perhaps application is shutting down
if (!enable && d->waitHandle != nullptr)
d->unregisterWaitObject();
@@ -256,7 +256,7 @@ void QWinEventNotifierPrivate::unregisterWaitObject()
static void CALLBACK wfsoCallback(void *context, BOOLEAN /*ignore*/)
{
QWinEventNotifierPrivate *nd = reinterpret_cast<QWinEventNotifierPrivate *>(context);
- QAbstractEventDispatcher *eventDispatcher = nd->threadData->eventDispatcher.loadRelaxed();
+ QAbstractEventDispatcher *eventDispatcher = nd->threadData.loadRelaxed()->eventDispatcher.loadRelaxed();
// Happens when Q(Core)Application is destroyed before QWinEventNotifier.
// https://bugreports.qt.io/browse/QTBUG-70214