Turn QThreadData::threadId into a QAtomicPointer

Solves a data race found by TSan.

Since thread and threadId are QAtomicPointer, I've removed the explicit
initialization in the QThreadData constructor

Task-number: QTBUG-58855
Change-Id: I4139d5f93dcb4b429ae9fffd14a34082f2683f76
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
This commit is contained in:
Thiago Macieira 2017-02-14 12:13:53 -08:00 committed by David Faure
parent 265db5ad9b
commit b58af67d7b
5 changed files with 17 additions and 17 deletions

View File

@ -3686,7 +3686,7 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i
continue; continue;
QObject * const receiver = c->receiver; QObject * const receiver = c->receiver;
const bool receiverInSameThread = currentThreadId == receiver->d_func()->threadData->threadId; const bool receiverInSameThread = currentThreadId == receiver->d_func()->threadData->threadId.load();
// determine if this connection should be sent immediately or // determine if this connection should be sent immediately or
// put into the event queue // put into the event queue

View File

@ -56,7 +56,7 @@ QT_BEGIN_NAMESPACE
*/ */
QThreadData::QThreadData(int initialRefCount) QThreadData::QThreadData(int initialRefCount)
: _ref(initialRefCount), loopLevel(0), scopeLevel(0), thread(0), threadId(0), : _ref(initialRefCount), loopLevel(0), scopeLevel(0),
eventDispatcher(0), eventDispatcher(0),
quitNow(false), canWait(true), isAdopted(false), requiresCoreApplication(true) quitNow(false), canWait(true), isAdopted(false), requiresCoreApplication(true)
{ {

View File

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

View File

@ -255,7 +255,7 @@ QThreadData *QThreadData::current(bool createIfNecessary)
} }
data->deref(); data->deref();
data->isAdopted = true; data->isAdopted = true;
data->threadId = to_HANDLE(pthread_self()); data->threadId.store(to_HANDLE(pthread_self()));
if (!QCoreApplicationPrivate::theMainThread) if (!QCoreApplicationPrivate::theMainThread)
QCoreApplicationPrivate::theMainThread = data->thread.load(); QCoreApplicationPrivate::theMainThread = data->thread.load();
} }
@ -335,7 +335,7 @@ void *QThreadPrivate::start(void *arg)
thr->d_func()->setPriority(QThread::Priority(thr->d_func()->priority & ~ThreadPriorityResetFlag)); thr->d_func()->setPriority(QThread::Priority(thr->d_func()->priority & ~ThreadPriorityResetFlag));
} }
data->threadId = to_HANDLE(pthread_self()); data->threadId.store(to_HANDLE(pthread_self()));
set_thread_data(data); set_thread_data(data);
data->ref(); data->ref();
@ -352,7 +352,7 @@ void *QThreadPrivate::start(void *arg)
// sets the name of the current thread. // sets the name of the current thread.
QString objectName = thr->objectName(); QString objectName = thr->objectName();
pthread_t thread_id = from_HANDLE<pthread_t>(data->threadId); pthread_t thread_id = from_HANDLE<pthread_t>(data->threadId.load());
if (Q_LIKELY(objectName.isEmpty())) if (Q_LIKELY(objectName.isEmpty()))
setCurrentThreadName(thread_id, thr->metaObject()->className()); setCurrentThreadName(thread_id, thr->metaObject()->className());
else else
@ -651,7 +651,7 @@ void QThread::start(Priority priority)
#endif #endif
code = pthread_create(&threadId, &attr, QThreadPrivate::start, this); code = pthread_create(&threadId, &attr, QThreadPrivate::start, this);
} }
d->data->threadId = to_HANDLE(threadId); d->data->threadId.store(to_HANDLE(threadId));
pthread_attr_destroy(&attr); pthread_attr_destroy(&attr);
@ -660,7 +660,7 @@ void QThread::start(Priority priority)
d->running = false; d->running = false;
d->finished = false; d->finished = false;
d->data->threadId = 0; d->data->threadId.store(nullptr);
} }
} }
@ -670,10 +670,10 @@ void QThread::terminate()
Q_D(QThread); Q_D(QThread);
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
if (!d->data->threadId) if (!d->data->threadId.load())
return; return;
int code = pthread_cancel(from_HANDLE<pthread_t>(d->data->threadId)); int code = pthread_cancel(from_HANDLE<pthread_t>(d->data->threadId.load()));
if (code) { if (code) {
qWarning("QThread::start: Thread termination error: %s", qWarning("QThread::start: Thread termination error: %s",
qPrintable(qt_error_string((code)))); qPrintable(qt_error_string((code))));
@ -686,7 +686,7 @@ bool QThread::wait(unsigned long time)
Q_D(QThread); Q_D(QThread);
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
if (from_HANDLE<pthread_t>(d->data->threadId) == pthread_self()) { if (from_HANDLE<pthread_t>(d->data->threadId.load()) == pthread_self()) {
qWarning("QThread::wait: Thread tried to wait on itself"); qWarning("QThread::wait: Thread tried to wait on itself");
return false; return false;
} }
@ -728,7 +728,7 @@ void QThreadPrivate::setPriority(QThread::Priority threadPriority)
int sched_policy; int sched_policy;
sched_param param; sched_param param;
if (pthread_getschedparam(from_HANDLE<pthread_t>(data->threadId), &sched_policy, &param) != 0) { if (pthread_getschedparam(from_HANDLE<pthread_t>(data->threadId.load()), &sched_policy, &param) != 0) {
// failed to get the scheduling policy, don't bother setting // failed to get the scheduling policy, don't bother setting
// the priority // the priority
qWarning("QThread::setPriority: Cannot get scheduler parameters"); qWarning("QThread::setPriority: Cannot get scheduler parameters");
@ -744,15 +744,15 @@ void QThreadPrivate::setPriority(QThread::Priority threadPriority)
} }
param.sched_priority = prio; param.sched_priority = prio;
int status = pthread_setschedparam(from_HANDLE<pthread_t>(data->threadId), sched_policy, &param); int status = pthread_setschedparam(from_HANDLE<pthread_t>(data->threadId.load()), sched_policy, &param);
# ifdef SCHED_IDLE # ifdef SCHED_IDLE
// were we trying to set to idle priority and failed? // were we trying to set to idle priority and failed?
if (status == -1 && sched_policy == SCHED_IDLE && errno == EINVAL) { if (status == -1 && sched_policy == SCHED_IDLE && errno == EINVAL) {
// reset to lowest priority possible // reset to lowest priority possible
pthread_getschedparam(from_HANDLE<pthread_t>(data->threadId), &sched_policy, &param); pthread_getschedparam(from_HANDLE<pthread_t>(data->threadId.load()), &sched_policy, &param);
param.sched_priority = sched_get_priority_min(sched_policy); param.sched_priority = sched_get_priority_min(sched_policy);
pthread_setschedparam(from_HANDLE<pthread_t>(data->threadId), sched_policy, &param); pthread_setschedparam(from_HANDLE<pthread_t>(data->threadId.load()), sched_policy, &param);
} }
# else # else
Q_UNUSED(status); Q_UNUSED(status);

View File

@ -137,7 +137,7 @@ QThreadData *QThreadData::current(bool createIfNecessary)
} }
threadData->deref(); threadData->deref();
threadData->isAdopted = true; threadData->isAdopted = true;
threadData->threadId = reinterpret_cast<Qt::HANDLE>(quintptr(GetCurrentThreadId())); threadData->threadId.store(reinterpret_cast<Qt::HANDLE>(quintptr(GetCurrentThreadId())));
if (!QCoreApplicationPrivate::theMainThread) { if (!QCoreApplicationPrivate::theMainThread) {
QCoreApplicationPrivate::theMainThread = threadData->thread.load(); QCoreApplicationPrivate::theMainThread = threadData->thread.load();
@ -351,7 +351,7 @@ unsigned int __stdcall QT_ENSURE_STACK_ALIGNED_FOR_SSE QThreadPrivate::start(voi
qt_create_tls(); qt_create_tls();
TlsSetValue(qt_current_thread_data_tls_index, data); TlsSetValue(qt_current_thread_data_tls_index, data);
data->threadId = reinterpret_cast<Qt::HANDLE>(quintptr(GetCurrentThreadId())); data->threadId.store(reinterpret_cast<Qt::HANDLE>(quintptr(GetCurrentThreadId())));
QThread::setTerminationEnabled(false); QThread::setTerminationEnabled(false);