From 13ca61fcfdc53a6a06ae6f409ae0d9e17919336c Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Thu, 15 Apr 2010 13:55:38 +0200 Subject: Fix tst_QEventLoop::exec() regression introduced by commit 816523117bc00cfeb17e347f7fe5f11278a5e871 The quitNow flag needs to be reset at the beginning of exec() to allow exec() to recurse and be run multiple times. This changes reverts commit 816523117bc00cfeb17e347f7fe5f11278a5e871 an introduces the QThreadPrivate::exited flag to fix the race between start() and exit(), and QThreadPrivate::returnCode to make sure exec() returns the code passed to exit(). Task-number: QTBUG-1184 Reviewed-by: olivier --- src/corelib/thread/qthread.cpp | 15 ++++++++++++--- src/corelib/thread/qthread_p.h | 3 +++ src/corelib/thread/qthread_unix.cpp | 2 +- src/corelib/thread/qthread_win.cpp | 2 +- tests/auto/qthread/tst_qthread.cpp | 32 ++++++++++++++++++-------------- 5 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp index cdcb65c..2c63dfc 100644 --- a/src/corelib/thread/qthread.cpp +++ b/src/corelib/thread/qthread.cpp @@ -173,7 +173,7 @@ void QAdoptedThread::run() */ QThreadPrivate::QThreadPrivate(QThreadData *d) - : QObjectPrivate(), running(false), finished(false), terminated(false), + : QObjectPrivate(), running(false), finished(false), terminated(false), exited(false), returnCode(-1), stackSize(0), priority(QThread::InheritPriority), data(d) { #if defined (Q_OS_UNIX) @@ -480,11 +480,18 @@ uint QThread::stackSize() const int QThread::exec() { Q_D(QThread); + QMutexLocker locker(&d->mutex); + d->data->quitNow = false; + if (d->exited) + return d->returnCode; + locker.unlock(); + QEventLoop eventLoop; int returnCode = eventLoop.exec(); - QMutexLocker locker(&d->mutex); - d->data->quitNow = false; + locker.relock(); + d->exited = false; + d->returnCode = -1; return returnCode; } @@ -511,6 +518,8 @@ void QThread::exit(int returnCode) { Q_D(QThread); QMutexLocker locker(&d->mutex); + d->exited = true; + d->returnCode = returnCode; d->data->quitNow = true; for (int i = 0; i < d->data->eventLoops.size(); ++i) { QEventLoop *eventLoop = d->data->eventLoops.at(i); diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h index 44eb8f8..d816aef 100644 --- a/src/corelib/thread/qthread_p.h +++ b/src/corelib/thread/qthread_p.h @@ -127,6 +127,9 @@ public: bool finished; bool terminated; + bool exited; + int returnCode; + uint stackSize; QThread::Priority priority; diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp index bd31d9c..6b34b5f 100644 --- a/src/corelib/thread/qthread_unix.cpp +++ b/src/corelib/thread/qthread_unix.cpp @@ -253,6 +253,7 @@ void *QThreadPrivate::start(void *arg) pthread_setspecific(current_thread_data_key, data); data->ref(); + data->quitNow = false; // ### TODO: allow the user to create a custom event dispatcher createEventDispatcher(data); @@ -494,7 +495,6 @@ void QThread::start(Priority priority) d->running = true; d->finished = false; d->terminated = false; - d->data->quitNow = false; pthread_attr_t attr; pthread_attr_init(&attr); diff --git a/src/corelib/thread/qthread_win.cpp b/src/corelib/thread/qthread_win.cpp index 9826dcb..37d5b87 100644 --- a/src/corelib/thread/qthread_win.cpp +++ b/src/corelib/thread/qthread_win.cpp @@ -298,6 +298,7 @@ unsigned int __stdcall QThreadPrivate::start(void *arg) QThread::setTerminationEnabled(false); + data->quitNow = false; // ### TODO: allow the user to create a custom event dispatcher createEventDispatcher(data); @@ -404,7 +405,6 @@ void QThread::start(Priority priority) d->running = true; d->finished = false; d->terminated = false; - d->data->quitNow = false; /* NOTE: we create the thread in the suspended state, set the diff --git a/tests/auto/qthread/tst_qthread.cpp b/tests/auto/qthread/tst_qthread.cpp index 871578e..9a4397e 100644 --- a/tests/auto/qthread/tst_qthread.cpp +++ b/tests/auto/qthread/tst_qthread.cpp @@ -174,8 +174,8 @@ public: void run() { + Simple_Thread::run(); if (object) { - Simple_Thread::run(); object->thread = this; object->code = code; QTimer::singleShot(100, object, SLOT(slot())); @@ -218,8 +218,8 @@ public: void run() { + Simple_Thread::run(); if (object) { - Simple_Thread::run(); object->thread = this; QTimer::singleShot(100, object, SLOT(slot())); } @@ -443,22 +443,24 @@ void tst_QThread::exit() thread2.code = 53; thread2.result = 0; thread2.start(); - thread2.exit(thread.code); + thread2.exit(thread2.code); + QMutexLocker locker2(&thread2.mutex); + thread2.cond.wait(locker2.mutex()); QVERIFY(thread2.wait(five_minutes)); - QCOMPARE(thread.result, thread.code); + QCOMPARE(thread2.result, thread2.code); } void tst_QThread::start() { QThread::Priority priorities[] = { - QThread::IdlePriority, - QThread::LowestPriority, - QThread::LowPriority, - QThread::NormalPriority, - QThread::HighPriority, - QThread::HighestPriority, - QThread::TimeCriticalPriority, - QThread::InheritPriority + QThread::IdlePriority, + QThread::LowestPriority, + QThread::LowPriority, + QThread::NormalPriority, + QThread::HighPriority, + QThread::HighestPriority, + QThread::TimeCriticalPriority, + QThread::InheritPriority }; const int prio_count = sizeof(priorities) / sizeof(QThread::Priority); @@ -514,8 +516,10 @@ void tst_QThread::quit() thread2.result = -1; thread2.start(); thread2.quit(); + QMutexLocker locker2(&thread2.mutex); + thread2.cond.wait(locker2.mutex()); QVERIFY(thread2.wait(five_minutes)); - QCOMPARE(thread.result, 0); + QCOMPARE(thread2.result, 0); } void tst_QThread::wait() @@ -692,7 +696,7 @@ void NativeThreadWrapper::start(FunctionPointer functionPointer, void *data) const int state = pthread_create(&nativeThreadHandle, 0, NativeThreadWrapper::runUnix, this); Q_UNUSED(state); #elif defined(Q_OS_WINCE) - nativeThreadHandle = CreateThread(NULL, 0 , (LPTHREAD_START_ROUTINE)NativeThreadWrapper::runWin , this, 0, NULL); + nativeThreadHandle = CreateThread(NULL, 0 , (LPTHREAD_START_ROUTINE)NativeThreadWrapper::runWin , this, 0, NULL); #elif defined Q_OS_WIN unsigned thrdid = 0; nativeThreadHandle = (Qt::HANDLE) _beginthreadex(NULL, 0, NativeThreadWrapper::runWin, this, 0, &thrdid); -- cgit v0.12