1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't store the pthread_t thread ID twice in QThread

Browse Source 
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>
This commit is contained in:
Thiago Macieira 2016-04-28 17:47:57 -07:00
parent 002112e805
commit 112e53fdc4
3 changed files with 18 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/corelib/thread/qthread.cpp
View File

@ -142,16 +142,12 @@ QThreadPrivate::QThreadPrivate(QThreadData *d)
exited(false), returnCode(-1),
stackSize(0), priority(QThread::InheritPriority), data(d)
{
#if defined (Q_OS_UNIX)
thread_id = 0;
#elif defined (Q_OS_WIN)
#if defined (Q_OS_WIN)
handle = 0;
# ifndef Q_OS_WINRT
id = 0;
# endif
waiters = 0;
#endif
#if defined (Q_OS_WIN)
terminationEnabled = true;
terminatePending = false;
#endif

1
src/corelib/thread/qthread_p.h
View File

@ -169,7 +169,6 @@ public:
static QThread *threadForId(int id);
#ifdef Q_OS_UNIX
pthread_t thread_id;
QWaitCondition thread_done;
static void *start(void *arg);

33
src/corelib/thread/qthread_unix.cpp
View File

@ -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, &param) != 0) {
if (pthread_getschedparam(reinterpret_cast<pthread_t>(data->threadId), &sched_policy, &param) != 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, &param);
int status = pthread_setschedparam(reinterpret_cast<pthread_t>(data->threadId), sched_policy, &param);
# 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, &param);
pthread_getschedparam(reinterpret_cast<pthread_t>(data->threadId), &sched_policy, &param);
param.sched_priority = sched_get_priority_min(sched_policy);
pthread_setschedparam(thread_id, sched_policy, &param);
pthread_setschedparam(reinterpret_cast<pthread_t>(data->threadId), sched_policy, &param);
}
# else
Q_UNUSED(status);