diff options
| author | Fabian Kosmale <fabian.kosmale@qt.io> | 2022-03-10 17:10:39 +0100 |
|---|---|---|
| committer | Fabian Kosmale <fabian.kosmale@qt.io> | 2022-05-24 18:15:41 +0200 |
| commit | 718d680579508284f7617a4e7db24d140438478d (patch) | |
| tree | 60b9954a5286c2b8988fb60f0f8dffa6d31d2215 /src/corelib/thread/qthread.cpp | |
| parent | 1221cd1f1b91610427f9037baaca0a0fda5a3d97 (diff) | |
Fix race conditions in moveToThread
Amends ba6c1d2785ca6d8a8b162abcd9d978ab0c52ea2d, which made
m_statusOrPendingObjects already atomic, but did not handle concurrent
deletion/push_back of the pendingObjects vector correctly.
We use the existing lock in QThreadPrivate to prevent data races.
Pick-to: 6.2 6.3
Fixes: QTBUG-101681
Change-Id: I0b440fee6ec270d762e6700a4fe74f28b19e75e8
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Diffstat (limited to 'src/corelib/thread/qthread.cpp')
| -rw-r--r-- | src/corelib/thread/qthread.cpp | 60 |
1 files changed, 52 insertions, 8 deletions
diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp index 735167ac4ec..d067bd06d3a 100644 --- a/src/corelib/thread/qthread.cpp +++ b/src/corelib/thread/qthread.cpp @@ -160,7 +160,10 @@ QThreadPrivate::QThreadPrivate(QThreadData *d) QThreadPrivate::~QThreadPrivate() { - delete pendingObjectsWithBindingStatusChange(); + // access to m_statusOrPendingObjects cannot race with anything + // unless there is already a potential use-after-free bug, as the + // thread is in the process of being destroyed + delete m_statusOrPendingObjects.list(); data->deref(); } @@ -503,6 +506,23 @@ uint QThread::stackSize() const } /*! + \internal + Transitions BindingStatusOrList to the binding status state. If we had a list of + pending objects, all objects get their reinitBindingStorageAfterThreadMove method + called, and afterwards, the list gets discarded. + */ +void QtPrivate::BindingStatusOrList::setStatusAndClearList(QBindingStatus *status) noexcept +{ + + if (auto pendingObjects = list()) { + for (auto obj: *pendingObjects) + QObjectPrivate::get(obj)->reinitBindingStorageAfterThreadMove(); + delete pendingObjects; + } + data = encodeBindingStatus(status); +} + +/*! Enters the event loop and waits until exit() is called, returning the value that was passed to exit(). The value returned is 0 if exit() is called via quit(). @@ -519,13 +539,9 @@ int QThread::exec() { Q_D(QThread); const auto status = QtPrivate::getBindingStatus(QtPrivate::QBindingStatusAccessToken{}); - if (auto pendingObjects = d->pendingObjectsWithBindingStatusChange()) { - for (auto obj: *pendingObjects) - QObjectPrivate::get(obj)->reinitBindingStorageAfterThreadMove(); - delete pendingObjects; - } - d->m_statusOrPendingObjects.storeRelease(quintptr(status)); + QMutexLocker locker(&d->mutex); + d->m_statusOrPendingObjects.setStatusAndClearList(status); d->data->quitNow = false; if (d->exited) { d->exited = false; @@ -542,6 +558,35 @@ int QThread::exec() return returnCode; } + +/*! + \internal + If BindingStatusOrList is already in the binding status state, this will + return that BindingStatus pointer. + Otherwise, \a object is added to the list, and we return nullptr. + The list is allocated if it does not already exist. + */ +QBindingStatus *QtPrivate::BindingStatusOrList::addObjectUnlessAlreadyStatus(QObject *object) +{ + + if (auto status = bindingStatus()) + return status; + List *objectList = list(); + if (!objectList) { + objectList = new List(); + objectList->reserve(8); + data = encodeList(objectList); + } + objectList->push_back(object); + return nullptr; +} + +QBindingStatus *QThreadPrivate::addObjectWithPendingBindingStatusChange(QObject *obj) +{ + QMutexLocker lock(&mutex); + return m_statusOrPendingObjects.addObjectUnlessAlreadyStatus(obj); +} + /*! \threadsafe Tells the thread's event loop to exit with a return code. @@ -926,7 +971,6 @@ QThreadPrivate::QThreadPrivate(QThreadData *d) : data(d ? d : new QThreadData) QThreadPrivate::~QThreadPrivate() { - delete pendingObjectsWithBindingStatusChange(); data->thread.storeRelease(nullptr); // prevent QThreadData from deleting the QThreadPrivate (again). delete data; } |