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

QThread: replace three booleans with one state variable

Browse Source 
There aren't 2^3 possible states for {running, isInFinish, finished}.
There are only three possible states for them.

There's a fourth case of NotStarted, which in my opinion is a design
flaw. It needs to exist so QThread().isFinished() is false, but in a
green field design we would be the same as QProcess: once a process/
thread finishes, it goes back to NotRunning.

I also made the Windows version set back to NotStarted when failing to
start, matching the Unix version.

Drive-by use NSDMI.

Change-Id: I262c3499666e4f4fbcfbfffd17cb3a48dad045dc
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Thiago Macieira 2024-04-30 18:49:27 -07:00
parent d05399c993
commit 452aaf340b
4 changed files with 46 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -127,8 +127,7 @@ QAdoptedThread::QAdoptedThread(QThreadData *data)
// thread should be running and not finished for the lifetime // thread should be running and not finished for the lifetime
// of the application (even if QCoreApplication goes away) // of the application (even if QCoreApplication goes away)
#if QT_CONFIG(thread) #if QT_CONFIG(thread)
d_func()->running = true; d_func()->threadState = QThreadPrivate::Running;
d_func()->finished = false;
init(); init();
d_func()->m_statusOrPendingObjects.setStatusAndClearList( d_func()->m_statusOrPendingObjects.setStatusAndClearList(
QtPrivate::getBindingStatus({})); QtPrivate::getBindingStatus({}));
@ -170,10 +169,7 @@ QScopedScopeLevelCounter::~QScopedScopeLevelCounter()
*/ */
QThreadPrivate::QThreadPrivate(QThreadData *d) QThreadPrivate::QThreadPrivate(QThreadData *d)
: QObjectPrivate(), running(false), finished(false), : QObjectPrivate(), data(d)
isInFinish(false), interruptionRequested(false),
exited(false), returnCode(-1),
stackSize(0), priority(QThread::InheritPriority), data(d)
{ {
// INTEGRITY doesn't support self-extending stack. The default stack size for // INTEGRITY doesn't support self-extending stack. The default stack size for
@ -492,12 +488,12 @@ QThread::~QThread()
Q_D(QThread); Q_D(QThread);
{ {
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
if (d->isInFinish) { if (d->threadState == QThreadPrivate::Finishing) {
locker.unlock(); locker.unlock();
wait(); wait();
locker.relock(); locker.relock();
} }
if (d->running && !d->finished && !d->data->isAdopted) if (d->threadState == QThreadPrivate::Running && !d->data->isAdopted)
qFatal("QThread: Destroyed while thread is still running"); qFatal("QThread: Destroyed while thread is still running");
d->data->thread.storeRelease(nullptr); d->data->thread.storeRelease(nullptr);
@ -514,7 +510,7 @@ bool QThread::isFinished() const
{ {
Q_D(const QThread); Q_D(const QThread);
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
return d->finished || d->isInFinish; return d->threadState >= QThreadPrivate::Finishing;
} }
/*! /*!
@ -527,7 +523,7 @@ bool QThread::isRunning() const
{ {
Q_D(const QThread); Q_D(const QThread);
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
return d->running && !d->isInFinish; return d->threadState == QThreadPrivate::Running;
} }
/*! /*!
@ -553,9 +549,9 @@ bool QThread::isRunning() const
void QThread::setStackSize(uint stackSize) void QThread::setStackSize(uint stackSize)
{ {
Q_D(QThread); Q_D(QThread);
QMutexLocker locker(&d->mutex); Q_ASSERT_X(!isRunning(), "QThread::setStackSize",
Q_ASSERT_X(!d->running, "QThread::setStackSize",
"cannot change stack size while the thread is running"); "cannot change stack size while the thread is running");
QMutexLocker locker(&d->mutex);
d->stackSize = stackSize; d->stackSize = stackSize;
} }
@ -769,7 +765,7 @@ void QThread::setPriority(Priority priority)
} }
Q_D(QThread); Q_D(QThread);
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
if (!d->running) { if (d->threadState != QThreadPrivate::Running) {
qWarning("QThread::setPriority: Cannot set priority, thread is not running"); qWarning("QThread::setPriority: Cannot set priority, thread is not running");
return; return;
} }
@ -1223,7 +1219,7 @@ void QThread::requestInterruption()
return; return;
} }
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
if (!d->running || d->finished || d->isInFinish) if (d->threadState != QThreadPrivate::Running)
return; return;
d->interruptionRequested.store(true, std::memory_order_relaxed); d->interruptionRequested.store(true, std::memory_order_relaxed);
} }
@ -1262,7 +1258,7 @@ bool QThread::isInterruptionRequested() const
return false; return false;
// slow path: if the flag is set, take into account run status: // slow path: if the flag is set, take into account run status:
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
return d->running && !d->finished && !d->isInFinish; return d->threadState == QThreadPrivate::Running;
} }
/*! /*!

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

@ -184,16 +184,22 @@ public:
mutable QMutex mutex; mutable QMutex mutex;
QAtomicInt quitLockRef; QAtomicInt quitLockRef;
bool running; enum State : quint8 {
bool finished; // All state changes are imprecise
bool isInFinish; //when in QThreadPrivate::finish NotStarted = 0, // before start() or if failed to start
Running = 1, // in run()
Finishing = 2, // in QThreadPrivate::finish()
Finished = 3, // QThreadPrivate::finish() is done
};
State threadState = NotStarted;
bool exited = false;
std::atomic<bool> interruptionRequested; std::atomic<bool> interruptionRequested;
bool exited; int returnCode = -1;
int returnCode;
uint stackSize; uint stackSize = 0;
std::underlying_type_t<QThread::Priority> priority; std::underlying_type_t<QThread::Priority> priority = QThread::InheritPriority;
#ifdef Q_OS_UNIX #ifdef Q_OS_UNIX
QWaitCondition thread_done; QWaitCondition thread_done;
@ -226,7 +232,7 @@ public:
void deref() void deref()
{ {
if (!quitLockRef.deref() && running) { if (!quitLockRef.deref() && threadState == Running) {
QCoreApplication::instance()->postEvent(q_ptr, new QEvent(QEvent::Quit)); QCoreApplication::instance()->postEvent(q_ptr, new QEvent(QEvent::Quit));
} }
} }

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

@ -95,7 +95,7 @@ static void destroy_current_thread_data(void *p)
QThread *thread = data->thread.loadAcquire(); QThread *thread = data->thread.loadAcquire();
Q_ASSERT(thread); Q_ASSERT(thread);
QThreadPrivate *thread_p = static_cast<QThreadPrivate *>(QObjectPrivate::get(thread)); QThreadPrivate *thread_p = static_cast<QThreadPrivate *>(QObjectPrivate::get(thread));
Q_ASSERT(!thread_p->finished); Q_ASSERT(thread_p->threadState == QThreadPrivate::Running);
thread_p->finish(thread); thread_p->finish(thread);
} }
data->deref(); data->deref();
@ -347,7 +347,7 @@ void QThreadPrivate::finish(void *arg)
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
d->isInFinish = true; d->threadState = QThreadPrivate::Finishing;
d->priority = QThread::InheritPriority; d->priority = QThread::InheritPriority;
void *data = &d->data->tls; void *data = &d->data->tls;
locker.unlock(); locker.unlock();
@ -366,11 +366,9 @@ void QThreadPrivate::finish(void *arg)
locker.relock(); locker.relock();
} }
d->running = false; d->threadState = QThreadPrivate::Finished;
d->finished = true;
d->interruptionRequested.store(false, std::memory_order_relaxed); d->interruptionRequested.store(false, std::memory_order_relaxed);
d->isInFinish = false;
d->data->threadId.storeRelaxed(nullptr); d->data->threadId.storeRelaxed(nullptr);
d->thread_done.wakeAll(); d->thread_done.wakeAll();
@ -629,14 +627,13 @@ void QThread::start(Priority priority)
Q_D(QThread); Q_D(QThread);
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
if (d->isInFinish) if (d->threadState == QThreadPrivate::Finishing)
d->thread_done.wait(locker.mutex()); d->thread_done.wait(locker.mutex());
if (d->running) if (d->threadState == QThreadPrivate::Running)
return; return;
d->running = true; d->threadState = QThreadPrivate::Running;
d->finished = false;
d->returnCode = 0; d->returnCode = 0;
d->exited = false; d->exited = false;
d->interruptionRequested.store(false, std::memory_order_relaxed); d->interruptionRequested.store(false, std::memory_order_relaxed);
@ -702,8 +699,7 @@ void QThread::start(Priority priority)
// we failed to set the stacksize, and as the documentation states, // we failed to set the stacksize, and as the documentation states,
// the thread will fail to run... // the thread will fail to run...
d->running = false; d->threadState = QThreadPrivate::NotStarted;
d->finished = false;
return; return;
} }
} }
@ -736,8 +732,7 @@ void QThread::start(Priority priority)
if (code) { if (code) {
qErrnoWarning(code, "QThread::start: Thread creation error"); qErrnoWarning(code, "QThread::start: Thread creation error");
d->running = false; d->threadState = QThreadPrivate::NotStarted;
d->finished = false;
d->data->threadId.storeRelaxed(nullptr); d->data->threadId.storeRelaxed(nullptr);
} }
} }
@ -768,10 +763,10 @@ bool QThread::wait(QDeadlineTimer deadline)
return false; return false;
} }
if (d->finished || !d->running) if (d->threadState == QThreadPrivate::NotStarted)
return true; return true;
while (d->running) { while (d->threadState != QThreadPrivate::Finished) {
if (!d->thread_done.wait(locker.mutex(), deadline)) if (!d->thread_done.wait(locker.mutex(), deadline))
return false; return false;
} }

26
src/corelib/thread/qthread_win.cpp
View File

@ -208,7 +208,7 @@ DWORD WINAPI qt_adopted_thread_watcher_function(LPVOID)
Q_ASSERT(thread); Q_ASSERT(thread);
auto thread_p = static_cast<QThreadPrivate *>(QObjectPrivate::get(thread)); auto thread_p = static_cast<QThreadPrivate *>(QObjectPrivate::get(thread));
Q_UNUSED(thread_p); Q_UNUSED(thread_p);
Q_ASSERT(!thread_p->finished); Q_ASSERT(thread_p->threadState == QThreadPrivate::Running);
QThreadPrivate::finish(thread); QThreadPrivate::finish(thread);
} }
data->deref(); data->deref();
@ -290,7 +290,7 @@ void QThreadPrivate::finish(void *arg, bool lockAnyway) noexcept
QThreadPrivate *d = thr->d_func(); QThreadPrivate *d = thr->d_func();
QMutexLocker locker(lockAnyway ? &d->mutex : nullptr); QMutexLocker locker(lockAnyway ? &d->mutex : nullptr);
d->isInFinish = true; d->threadState = QThreadPrivate::Finishing;
d->priority = QThread::InheritPriority; d->priority = QThread::InheritPriority;
void **tls_data = reinterpret_cast<void **>(&d->data->tls); void **tls_data = reinterpret_cast<void **>(&d->data->tls);
if (lockAnyway) if (lockAnyway)
@ -313,9 +313,7 @@ void QThreadPrivate::finish(void *arg, bool lockAnyway) noexcept
locker.relock(); locker.relock();
} }
d->running = false; d->threadState = QThreadPrivate::Finished;
d->finished = true;
d->isInFinish = false;
d->interruptionRequested.store(false, std::memory_order_relaxed); d->interruptionRequested.store(false, std::memory_order_relaxed);
if (!d->waiters) { if (!d->waiters) {
@ -377,20 +375,19 @@ void QThread::start(Priority priority)
Q_D(QThread); Q_D(QThread);
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
if (d->isInFinish) { if (d->threadState == QThreadPrivate::Finishing) {
locker.unlock(); locker.unlock();
wait(); wait();
locker.relock(); locker.relock();
} }
if (d->running) if (d->threadState == QThreadPrivate::Running)
return; return;
// avoid interacting with the binding system // avoid interacting with the binding system
d->objectName = d->extraData ? d->extraData->objectName.valueBypassingBindings() d->objectName = d->extraData ? d->extraData->objectName.valueBypassingBindings()
: QString(); : QString();
d->running = true; d->threadState = QThreadPrivate::Running;
d->finished = false;
d->exited = false; d->exited = false;
d->returnCode = 0; d->returnCode = 0;
d->interruptionRequested.store(false, std::memory_order_relaxed); d->interruptionRequested.store(false, std::memory_order_relaxed);
@ -418,8 +415,7 @@ void QThread::start(Priority priority)
if (!d->handle) { if (!d->handle) {
qErrnoWarning("QThread::start: Failed to create thread"); qErrnoWarning("QThread::start: Failed to create thread");
d->running = false; d->threadState = QThreadPrivate::NotStarted;
d->finished = true;
return; return;
} }
@ -473,7 +469,7 @@ void QThread::terminate()
{ {
Q_D(QThread); Q_D(QThread);
QMutexLocker locker(&d->mutex); QMutexLocker locker(&d->mutex);
if (!d->running) if (d->threadState != QThreadPrivate::Running)
return; return;
if (!d->terminationEnabled) { if (!d->terminationEnabled) {
d->terminatePending = true; d->terminatePending = true;
@ -493,7 +489,7 @@ bool QThread::wait(QDeadlineTimer deadline)
qWarning("QThread::wait: Thread tried to wait on itself"); qWarning("QThread::wait: Thread tried to wait on itself");
return false; return false;
} }
if (d->finished || !d->running) if (d->threadState == QThreadPrivate::NotStarted || d->threadState == QThreadPrivate::Finished)
return true; return true;
++d->waiters; ++d->waiters;
@ -516,13 +512,13 @@ bool QThread::wait(QDeadlineTimer deadline)
locker.mutex()->lock(); locker.mutex()->lock();
--d->waiters; --d->waiters;
if (ret && !d->finished) { if (ret && d->threadState < QThreadPrivate::Finished) {
// thread was terminated by someone else // thread was terminated by someone else
QThreadPrivate::finish(this, false); QThreadPrivate::finish(this, false);
} }
if (d->finished && !d->waiters) { if (d->threadState == QThreadPrivate::Finished && !d->waiters) {
CloseHandle(d->handle); CloseHandle(d->handle);
d->handle = 0; d->handle = 0;
} }