Fix two data races in QThread/QThreadData

* theMainThread is written by the main thread and read by
   QThreadData::~QThreadData() (any managed thread)

* QThreadData::thread is written by QThread::~QThread (in the parent thread)
  and read+written by QThreadData::~QThreadData (in the managed thread).
  This can happen because QThreadData is refcounted so the managed
  thread (which derefs it) races with the parent thread (which sets it to 0).

Change-Id: I72de793716391a0937254cda6b4328fcad5060c7
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
David Faure 2015-08-01 15:50:00 +02:00
parent 71df75966d
commit ec6556a2b9
7 changed files with 14 additions and 14 deletions

View File

@ -460,8 +460,8 @@ QCoreApplicationPrivate::QCoreApplicationPrivate(int &aargc, char **aargv, uint
qt_application_thread_id = QThread::currentThreadId();
# endif
// note: this call to QThread::currentThread() may end up setting theMainThread!
if (QThread::currentThread() != theMainThread)
QThread *cur = QThread::currentThread(); // note: this may end up setting theMainThread!
if (cur != theMainThread)
qWarning("WARNING: QApplication was not created in the main() thread.");
#endif
}
@ -531,11 +531,11 @@ void QCoreApplicationPrivate::eventDispatcherReady()
{
}
QThread *QCoreApplicationPrivate::theMainThread = 0;
QBasicAtomicPointer<QThread> QCoreApplicationPrivate::theMainThread = Q_BASIC_ATOMIC_INITIALIZER(0);
QThread *QCoreApplicationPrivate::mainThread()
{
Q_ASSERT(theMainThread != 0);
return theMainThread;
Q_ASSERT(theMainThread.load() != 0);
return theMainThread.load();
}
void QCoreApplicationPrivate::checkReceiverThread(QObject *receiver)
@ -2671,7 +2671,7 @@ bool QCoreApplication::hasPendingEvents()
QAbstractEventDispatcher *QCoreApplication::eventDispatcher()
{
if (QCoreApplicationPrivate::theMainThread)
return QCoreApplicationPrivate::theMainThread->eventDispatcher();
return QCoreApplicationPrivate::theMainThread.load()->eventDispatcher();
return 0;
}

View File

@ -105,7 +105,7 @@ public:
}
void maybeQuit();
static QThread *theMainThread;
static QBasicAtomicPointer<QThread> theMainThread;
static QThread *mainThread();
static void sendPostedEvents(QObject *receiver, int event_type, QThreadData *data);

View File

@ -1468,7 +1468,7 @@ void QObject::moveToThread(QThread *targetThread)
} else if (d->threadData != currentData) {
qWarning("QObject::moveToThread: Current thread (%p) is not the object's thread (%p).\n"
"Cannot move to target thread (%p)\n",
currentData->thread, d->threadData->thread, targetData ? targetData->thread : Q_NULLPTR);
currentData->thread.load(), d->threadData->thread.load(), targetData ? targetData->thread.load() : Q_NULLPTR);
#ifdef Q_OS_MAC
qWarning("You might be loading two sets of Qt binaries into the same process. "

View File

@ -269,7 +269,7 @@ public:
QStack<QEventLoop *> eventLoops;
QPostEventList postEventList;
QThread *thread;
QAtomicPointer<QThread> thread;
Qt::HANDLE threadId;
QAtomicPointer<QAbstractEventDispatcher> eventDispatcher;
QVector<void *> tls;

View File

@ -225,7 +225,7 @@ QThreadData *QThreadData::current(bool createIfNecessary)
data->isAdopted = true;
data->threadId = (Qt::HANDLE)pthread_self();
if (!QCoreApplicationPrivate::theMainThread)
QCoreApplicationPrivate::theMainThread = data->thread;
QCoreApplicationPrivate::theMainThread = data->thread.load();
}
return data;
}

View File

@ -121,7 +121,7 @@ QThreadData *QThreadData::current(bool createIfNecessary)
threadData->threadId = reinterpret_cast<Qt::HANDLE>(GetCurrentThreadId());
if (!QCoreApplicationPrivate::theMainThread) {
QCoreApplicationPrivate::theMainThread = threadData->thread;
QCoreApplicationPrivate::theMainThread = threadData->thread.load();
// TODO: is there a way to reflect the branch's behavior using
// WinRT API?
} else {

View File

@ -121,7 +121,7 @@ void **QThreadStorageData::get() const
DEBUG_MSG("QThreadStorageData: Returning storage %d, data %p, for thread %p",
id,
*v,
data->thread);
data->thread.load());
return *v ? v : 0;
}
@ -143,7 +143,7 @@ void **QThreadStorageData::set(void *p)
DEBUG_MSG("QThreadStorageData: Deleting previous storage %d, data %p, for thread %p",
id,
value,
data->thread);
data->thread.load());
QMutexLocker locker(&destructorsMutex);
DestructorMap *destr = destructors();
@ -159,7 +159,7 @@ void **QThreadStorageData::set(void *p)
// store new data
value = p;
DEBUG_MSG("QThreadStorageData: Set storage %d for thread %p to %p", id, data->thread, p);
DEBUG_MSG("QThreadStorageData: Set storage %d for thread %p to %p", id, data->thread.load(), p);
return &value;
}