From b58af67d7b5f22eaf2916b91d8a7060781203561 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 14 Feb 2017 12:13:53 -0800 Subject: 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 --- src/corelib/thread/qthread.cpp | 2 +- src/corelib/thread/qthread_p.h | 2 +- src/corelib/thread/qthread_unix.cpp | 24 ++++++++++++------------ src/corelib/thread/qthread_win.cpp | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) (limited to 'src/corelib/thread') diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp index 7118ad5c9b..c08038cf90 100644 --- a/src/corelib/thread/qthread.cpp +++ b/src/corelib/thread/qthread.cpp @@ -56,7 +56,7 @@ QT_BEGIN_NAMESPACE */ QThreadData::QThreadData(int initialRefCount) - : _ref(initialRefCount), loopLevel(0), scopeLevel(0), thread(0), threadId(0), + : _ref(initialRefCount), loopLevel(0), scopeLevel(0), eventDispatcher(0), quitNow(false), canWait(true), isAdopted(false), requiresCoreApplication(true) { diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h index 37eca9c612..885b4c0c1e 100644 --- a/src/corelib/thread/qthread_p.h +++ b/src/corelib/thread/qthread_p.h @@ -284,7 +284,7 @@ public: QStack eventLoops; QPostEventList postEventList; QAtomicPointer thread; - Qt::HANDLE threadId; + QAtomicPointer threadId; QAtomicPointer eventDispatcher; QVector tls; FlaggedDebugSignatures flaggedSignatures; diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp index ba5f2dca95..6d00aa42ff 100644 --- a/src/corelib/thread/qthread_unix.cpp +++ b/src/corelib/thread/qthread_unix.cpp @@ -255,7 +255,7 @@ QThreadData *QThreadData::current(bool createIfNecessary) } data->deref(); data->isAdopted = true; - data->threadId = to_HANDLE(pthread_self()); + data->threadId.store(to_HANDLE(pthread_self())); if (!QCoreApplicationPrivate::theMainThread) 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)); } - data->threadId = to_HANDLE(pthread_self()); + data->threadId.store(to_HANDLE(pthread_self())); set_thread_data(data); data->ref(); @@ -352,7 +352,7 @@ void *QThreadPrivate::start(void *arg) // sets the name of the current thread. QString objectName = thr->objectName(); - pthread_t thread_id = from_HANDLE(data->threadId); + pthread_t thread_id = from_HANDLE(data->threadId.load()); if (Q_LIKELY(objectName.isEmpty())) setCurrentThreadName(thread_id, thr->metaObject()->className()); else @@ -651,7 +651,7 @@ void QThread::start(Priority priority) #endif 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); @@ -660,7 +660,7 @@ void QThread::start(Priority priority) d->running = false; d->finished = false; - d->data->threadId = 0; + d->data->threadId.store(nullptr); } } @@ -670,10 +670,10 @@ void QThread::terminate() Q_D(QThread); QMutexLocker locker(&d->mutex); - if (!d->data->threadId) + if (!d->data->threadId.load()) return; - int code = pthread_cancel(from_HANDLE(d->data->threadId)); + int code = pthread_cancel(from_HANDLE(d->data->threadId.load())); if (code) { qWarning("QThread::start: Thread termination error: %s", qPrintable(qt_error_string((code)))); @@ -686,7 +686,7 @@ bool QThread::wait(unsigned long time) Q_D(QThread); QMutexLocker locker(&d->mutex); - if (from_HANDLE(d->data->threadId) == pthread_self()) { + if (from_HANDLE(d->data->threadId.load()) == pthread_self()) { qWarning("QThread::wait: Thread tried to wait on itself"); return false; } @@ -728,7 +728,7 @@ void QThreadPrivate::setPriority(QThread::Priority threadPriority) int sched_policy; sched_param param; - if (pthread_getschedparam(from_HANDLE(data->threadId), &sched_policy, ¶m) != 0) { + if (pthread_getschedparam(from_HANDLE(data->threadId.load()), &sched_policy, ¶m) != 0) { // failed to get the scheduling policy, don't bother setting // the priority qWarning("QThread::setPriority: Cannot get scheduler parameters"); @@ -744,15 +744,15 @@ void QThreadPrivate::setPriority(QThread::Priority threadPriority) } param.sched_priority = prio; - int status = pthread_setschedparam(from_HANDLE(data->threadId), sched_policy, ¶m); + int status = pthread_setschedparam(from_HANDLE(data->threadId.load()), sched_policy, ¶m); # ifdef SCHED_IDLE // were we trying to set to idle priority and failed? if (status == -1 && sched_policy == SCHED_IDLE && errno == EINVAL) { // reset to lowest priority possible - pthread_getschedparam(from_HANDLE(data->threadId), &sched_policy, ¶m); + pthread_getschedparam(from_HANDLE(data->threadId.load()), &sched_policy, ¶m); param.sched_priority = sched_get_priority_min(sched_policy); - pthread_setschedparam(from_HANDLE(data->threadId), sched_policy, ¶m); + pthread_setschedparam(from_HANDLE(data->threadId.load()), sched_policy, ¶m); } # else Q_UNUSED(status); diff --git a/src/corelib/thread/qthread_win.cpp b/src/corelib/thread/qthread_win.cpp index e6c70ecb55..ef7bfc511d 100644 --- a/src/corelib/thread/qthread_win.cpp +++ b/src/corelib/thread/qthread_win.cpp @@ -137,7 +137,7 @@ QThreadData *QThreadData::current(bool createIfNecessary) } threadData->deref(); threadData->isAdopted = true; - threadData->threadId = reinterpret_cast(quintptr(GetCurrentThreadId())); + threadData->threadId.store(reinterpret_cast(quintptr(GetCurrentThreadId()))); if (!QCoreApplicationPrivate::theMainThread) { QCoreApplicationPrivate::theMainThread = threadData->thread.load(); @@ -351,7 +351,7 @@ unsigned int __stdcall QT_ENSURE_STACK_ALIGNED_FOR_SSE QThreadPrivate::start(voi qt_create_tls(); TlsSetValue(qt_current_thread_data_tls_index, data); - data->threadId = reinterpret_cast(quintptr(GetCurrentThreadId())); + data->threadId.store(reinterpret_cast(quintptr(GetCurrentThreadId()))); QThread::setTerminationEnabled(false); -- cgit v1.2.3