From 733923c81cd313a02976c026484654e4501b3527 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 10 May 2021 17:34:20 +0200 Subject: [PATCH] Add a safer way to use QThreadPool::reserveThread Add startOnReservedThread that specifically releases a reserved thread and uses it atomically for a given task. This can make a positive number of reserved threads work. Change-Id: I4bd1dced24bb46fcb365f12cbc9c7905dc66cdf1 Reviewed-by: David Faure --- src/corelib/thread/qthreadpool.cpp | 51 ++++++++++++++++++ src/corelib/thread/qthreadpool.h | 3 ++ .../thread/qthreadpool/tst_qthreadpool.cpp | 54 ++++++++++++++++++- 3 files changed, 106 insertions(+), 2 deletions(-) diff --git a/src/corelib/thread/qthreadpool.cpp b/src/corelib/thread/qthreadpool.cpp index 96eae6f3f3..b067522ed2 100644 --- a/src/corelib/thread/qthreadpool.cpp +++ b/src/corelib/thread/qthreadpool.cpp @@ -770,6 +770,57 @@ void QThreadPool::releaseThread() d->tryToStartMoreThreads(); } +/*! + Releases a thread previously reserved with reserveThread() and uses it + to run \a runnable. + + Note that the thread pool takes ownership of the \a runnable if + \l{QRunnable::autoDelete()}{runnable->autoDelete()} returns \c true, + and the \a runnable will be deleted automatically by the thread + pool after the \l{QRunnable::run()}{runnable->run()} returns. If + \l{QRunnable::autoDelete()}{runnable->autoDelete()} returns \c false, + ownership of \a runnable remains with the caller. Note that + changing the auto-deletion on \a runnable after calling this + functions results in undefined behavior. + + \note Calling this when no threads are reserved results in + undefined behavior. + + \since 6.3 + \sa reserveThread(), start() +*/ +void QThreadPool::startOnReservedThread(QRunnable *runnable) +{ + if (!runnable) + return releaseThread(); + + Q_D(QThreadPool); + QMutexLocker locker(&d->mutex); + Q_ASSERT(d->reservedThreads > 0); + --d->reservedThreads; + + if (!d->tryStart(runnable)) { + // This can only happen if we reserved max threads, + // and something took the one minimum thread. + d->enqueueTask(runnable, INT_MAX); + } +} + +/*! + \overload + \since 6.3 + + Releases a thread previously reserved with reserveThread() and uses it + to run \a functionToRun. +*/ +void QThreadPool::startOnReservedThread(std::function functionToRun) +{ + if (!functionToRun) + return releaseThread(); + + startOnReservedThread(QRunnable::create(std::move(functionToRun))); +} + /*! Waits up to \a msecs milliseconds for all threads to exit and removes all threads from the thread pool. Returns \c true if all threads were removed; diff --git a/src/corelib/thread/qthreadpool.h b/src/corelib/thread/qthreadpool.h index f3d7e2254b..853055cb30 100644 --- a/src/corelib/thread/qthreadpool.h +++ b/src/corelib/thread/qthreadpool.h @@ -75,6 +75,9 @@ public: void start(std::function functionToRun, int priority = 0); bool tryStart(std::function functionToRun); + void startOnReservedThread(QRunnable *runnable); + void startOnReservedThread(std::function functionToRun); + int expiryTimeout() const; void setExpiryTimeout(int expiryTimeout); diff --git a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp index 3ce4ee09b3..404ebb68a1 100644 --- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp +++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp @@ -89,6 +89,7 @@ private slots: void releaseThread_data(); void releaseThread(); void reserveAndStart(); + void reserveAndStart2(); void releaseAndBlock(); void start(); void tryStart(); @@ -729,17 +730,66 @@ void tst_QThreadPool::reserveAndStart() // QTBUG-21051 // start() will wake up the waiting thread. threadpool->start(&task); QTRY_COMPARE(threadpool->activeThreadCount(), 2); + QTRY_COMPARE(task.count.loadRelaxed(), 2); + WaitingTask task2; + // startOnReservedThread() will try to take the reserved task, but end up waiting instead + threadpool->startOnReservedThread(&task2); + QTRY_COMPARE(threadpool->activeThreadCount(), 1); task.waitForStarted.acquire(); task.waitBeforeDone.release(); - QTRY_COMPARE(task.count.loadRelaxed(), 2); QTRY_COMPARE(threadpool->activeThreadCount(), 1); + task2.waitForStarted.acquire(); + task2.waitBeforeDone.release(); - threadpool->releaseThread(); QTRY_COMPARE(threadpool->activeThreadCount(), 0); threadpool->setMaxThreadCount(savedLimit); } +void tst_QThreadPool::reserveAndStart2() +{ + class WaitingTask : public QRunnable + { + public: + QSemaphore waitBeforeDone; + + WaitingTask() { setAutoDelete(false); } + + void run() override + { + waitBeforeDone.acquire(); + } + }; + + // Set up + QThreadPool *threadpool = QThreadPool::globalInstance(); + int savedLimit = threadpool->maxThreadCount(); + threadpool->setMaxThreadCount(2); + + // reserve + threadpool->reserveThread(); + + // start two task, to get a running thread and one queued + WaitingTask task1, task2, task3; + threadpool->start(&task1); + // one running thread, one reserved: + QCOMPARE(threadpool->activeThreadCount(), 2); + // task2 starts queued + threadpool->start(&task2); + QCOMPARE(threadpool->activeThreadCount(), 2); + // startOnReservedThread() will take the reserved thread however, bypassing the queue + threadpool->startOnReservedThread(&task3); + // two running threads, none reserved: + QCOMPARE(threadpool->activeThreadCount(), 2); + task3.waitBeforeDone.release(); + // task3 can finish even if all other tasks are blocking + // then task2 will use the previously reserved thread + task2.waitBeforeDone.release(); + QTRY_COMPARE(threadpool->activeThreadCount(), 1); + task1.waitBeforeDone.release(); + QTRY_COMPARE(threadpool->activeThreadCount(), 0); +} + void tst_QThreadPool::releaseAndBlock() { class WaitingTask : public QRunnable