diff options
author | Thiago Macieira <thiago.macieira@intel.com> | 2016-04-28 17:47:57 -0700 |
---|---|---|
committer | Thiago Macieira <thiago.macieira@intel.com> | 2016-04-30 03:00:33 +0000 |
commit | 112e53fdc4e46a5e94cb2d575d132e2015694407 (patch) | |
tree | 999de17175e00356d1a684b37a0298789a7eea62 /src/corelib/thread/qthread_unix.cpp | |
parent | 002112e80516a29efbb6cef721d74c5fc39fc19d (diff) |
Don't store the pthread_t thread ID twice in QThread
It was being stored once in QThreadPrivate and once in QThreadData, with
the latter being hidden as a Qt::HANDLE. Besides saving a little bit of
memory, this also solves a small data race condition that arises from
trying to connect a signal to an object moved to that thread and then
emit that signal shortly after the thread starts. Before this patch,
QThreadData::threadId was initialized only by QThreadPrivate::start(),
which meant that we were racing that initialization with this check in
QMetaObject::activate:
const bool receiverInSameThread = currentThreadId == receiver->d_func()->threadData->threadId;
Task-number: QTBUG-52337
Change-Id: Ifea6e497f11a461db432ffff1449ae01f1099aae
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@theqtcompany.com>
Diffstat (limited to 'src/corelib/thread/qthread_unix.cpp')
-rw-r--r-- | src/corelib/thread/qthread_unix.cpp | 33 |
1 files changed, 17 insertions, 16 deletions
diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp index af4ce7c59e..3fc0ebfbb6 100644 --- a/src/corelib/thread/qthread_unix.cpp +++ b/src/corelib/thread/qthread_unix.cpp @@ -105,6 +105,8 @@ QT_BEGIN_NAMESPACE #ifndef QT_NO_THREAD +Q_STATIC_ASSERT(sizeof(pthread_t) == sizeof(Qt::HANDLE)); + enum { ThreadPriorityResetFlag = 0x80000000 }; #if defined(Q_OS_LINUX) && defined(__GLIBC__) && (defined(Q_CC_GNU) || defined(Q_CC_INTEL)) && !defined(QT_LINUXBASE) @@ -234,8 +236,6 @@ QThreadData *QThreadData::current(bool createIfNecessary) void QAdoptedThread::init() { - Q_D(QThread); - d->thread_id = pthread_self(); } /* @@ -325,10 +325,11 @@ void *QThreadPrivate::start(void *arg) // sets the name of the current thread. QString objectName = thr->objectName(); + pthread_t thread_id = reinterpret_cast<pthread_t>(data->threadId); if (Q_LIKELY(objectName.isEmpty())) - setCurrentThreadName(thr->d_func()->thread_id, thr->metaObject()->className()); + setCurrentThreadName(thread_id, thr->metaObject()->className()); else - setCurrentThreadName(thr->d_func()->thread_id, objectName.toLocal8Bit()); + setCurrentThreadName(thread_id, objectName.toLocal8Bit()); } #endif @@ -369,7 +370,6 @@ void QThreadPrivate::finish(void *arg) locker.relock(); } - d->thread_id = 0; d->running = false; d->finished = true; d->interruptionRequested = false; @@ -615,15 +615,16 @@ void QThread::start(Priority priority) } int code = - pthread_create(&d->thread_id, &attr, QThreadPrivate::start, this); + pthread_create(reinterpret_cast<pthread_t *>(&d->data->threadId), &attr, + QThreadPrivate::start, this); if (code == EPERM) { // caller does not have permission to set the scheduling // parameters/policy #if defined(QT_HAS_THREAD_PRIORITY_SCHEDULING) pthread_attr_setinheritsched(&attr, PTHREAD_INHERIT_SCHED); #endif - code = - pthread_create(&d->thread_id, &attr, QThreadPrivate::start, this); + code = pthread_create(reinterpret_cast<pthread_t *>(&d->data->threadId), &attr, + QThreadPrivate::start, this); } pthread_attr_destroy(&attr); @@ -633,7 +634,7 @@ void QThread::start(Priority priority) d->running = false; d->finished = false; - d->thread_id = 0; + d->data->threadId = 0; } } @@ -643,10 +644,10 @@ void QThread::terminate() Q_D(QThread); QMutexLocker locker(&d->mutex); - if (!d->thread_id) + if (!d->data->threadId) return; - int code = pthread_cancel(d->thread_id); + int code = pthread_cancel(reinterpret_cast<pthread_t>(d->data->threadId)); if (code) { qWarning("QThread::start: Thread termination error: %s", qPrintable(qt_error_string((code)))); @@ -659,7 +660,7 @@ bool QThread::wait(unsigned long time) Q_D(QThread); QMutexLocker locker(&d->mutex); - if (d->thread_id == pthread_self()) { + if (reinterpret_cast<pthread_t>(d->data->threadId) == pthread_self()) { qWarning("QThread::wait: Thread tried to wait on itself"); return false; } @@ -701,7 +702,7 @@ void QThreadPrivate::setPriority(QThread::Priority threadPriority) int sched_policy; sched_param param; - if (pthread_getschedparam(thread_id, &sched_policy, ¶m) != 0) { + if (pthread_getschedparam(reinterpret_cast<pthread_t>(data->threadId), &sched_policy, ¶m) != 0) { // failed to get the scheduling policy, don't bother setting // the priority qWarning("QThread::setPriority: Cannot get scheduler parameters"); @@ -717,15 +718,15 @@ void QThreadPrivate::setPriority(QThread::Priority threadPriority) } param.sched_priority = prio; - int status = pthread_setschedparam(thread_id, sched_policy, ¶m); + int status = pthread_setschedparam(reinterpret_cast<pthread_t>(data->threadId), 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(thread_id, &sched_policy, ¶m); + pthread_getschedparam(reinterpret_cast<pthread_t>(data->threadId), &sched_policy, ¶m); param.sched_priority = sched_get_priority_min(sched_policy); - pthread_setschedparam(thread_id, sched_policy, ¶m); + pthread_setschedparam(reinterpret_cast<pthread_t>(data->threadId), sched_policy, ¶m); } # else Q_UNUSED(status); |