From 0e7e1c33965c43932e6c0228acc2f68259533482 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 11 Apr 2023 11:38:37 +0200 Subject: [PATCH] Take move-only functions for the threadpool We never copy the function so only need it to movable. Moves the functions to templates using the new QRunnable create version. [ChangeLog][QtCore][QThreadPool] Methods taking callable functions, can now take move-only lambdas. Fixes: QTBUG-112302 Change-Id: I2cb200f0abcf7e0fdbef0457fe2a6176764ad93d Reviewed-by: Marc Mutz --- src/corelib/compat/removed_api.cpp | 39 +++++++++++++++ src/corelib/thread/qrunnable.h | 36 +++++++++++-- src/corelib/thread/qthreadpool.cpp | 50 ++++++++----------- src/corelib/thread/qthreadpool.h | 33 ++++++++++++ .../thread/qthreadpool/tst_qthreadpool.cpp | 20 +++++++- 5 files changed, 144 insertions(+), 34 deletions(-) diff --git a/src/corelib/compat/removed_api.cpp b/src/corelib/compat/removed_api.cpp index 399d7b15f5..cd1c8691bf 100644 --- a/src/corelib/compat/removed_api.cpp +++ b/src/corelib/compat/removed_api.cpp @@ -524,6 +524,45 @@ qsizetype QString::toUcs4_helper(const ushort *uc, qsizetype length, uint *out) reinterpret_cast(out)); } +#if QT_CONFIG(thread) + +#include "qthreadpool.h" +#include "private/qthreadpool_p.h" + +void QThreadPool::start(std::function functionToRun, int priority) +{ + if (!functionToRun) + return; + start(QRunnable::create(std::move(functionToRun)), priority); +} + +bool QThreadPool::tryStart(std::function functionToRun) +{ + if (!functionToRun) + return false; + + Q_D(QThreadPool); + QMutexLocker locker(&d->mutex); + if (!d->allThreads.isEmpty() && d->areAllThreadsActive()) + return false; + + QRunnable *runnable = QRunnable::create(std::move(functionToRun)); + if (d->tryStart(runnable)) + return true; + delete runnable; + return false; +} + +void QThreadPool::startOnReservedThread(std::function functionToRun) +{ + if (!functionToRun) + return releaseThread(); + + startOnReservedThread(QRunnable::create(std::move(functionToRun))); +} + +#endif // QT_CONFIG(thread) + #include "qxmlstream.h" QStringView QXmlStreamAttributes::value(const QString &namespaceUri, const QString &name) const diff --git a/src/corelib/thread/qrunnable.h b/src/corelib/thread/qrunnable.h index d8473a7f5f..3b25b67836 100644 --- a/src/corelib/thread/qrunnable.h +++ b/src/corelib/thread/qrunnable.h @@ -4,7 +4,9 @@ #ifndef QRUNNABLE_H #define QRUNNABLE_H -#include +#include +#include + #include #include @@ -28,6 +30,8 @@ public: template = true> static QRunnable *create(Callable &&functionToRun); + static QRunnable *create(std::nullptr_t) = delete; + bool autoDelete() const { return m_autoDelete; } void setAutoDelete(bool autoDelete) { m_autoDelete = autoDelete; } @@ -35,7 +39,7 @@ protected: // Type erasure, to only instantiate a non-virtual class per Callable: class QGenericRunnableHelperBase { - using OpFn = void(*)(const QGenericRunnableHelperBase *); + using OpFn = void(*)(QGenericRunnableHelperBase *); OpFn runFn; OpFn destroyFn; protected: @@ -54,8 +58,8 @@ protected: template QGenericRunnableHelper(UniCallable &&functionToRun) noexcept : QGenericRunnableHelperBase( - [](const QGenericRunnableHelperBase *that) { static_cast(that)->m_functionToRun(); }, - [](const QGenericRunnableHelperBase *that) { delete static_cast(that); }), + [](QGenericRunnableHelperBase *that) { static_cast(that)->m_functionToRun(); }, + [](QGenericRunnableHelperBase *that) { delete static_cast(that); }), m_functionToRun(std::forward(functionToRun)) { } @@ -79,9 +83,33 @@ public: } }; +namespace QtPrivate { + +template +using is_function_pointer = std::conjunction, std::is_function>>; +template +struct is_std_function : std::false_type {}; +template +struct is_std_function> : std::true_type {}; + +} // namespace QtPrivate + template > QRunnable *QRunnable::create(Callable &&functionToRun) { + bool is_null = false; + if constexpr(QtPrivate::is_std_function>::value) + is_null = !functionToRun; + + if constexpr(QtPrivate::is_function_pointer>::value) { + const void *functionPtr = reinterpret_cast(functionToRun); + is_null = !functionPtr; + } + if (is_null) { + qWarning() << "Trying to create null QRunnable. This may stop working."; + return nullptr; + } + return new QGenericRunnable( new QGenericRunnableHelper>( std::forward(functionToRun))); diff --git a/src/corelib/thread/qthreadpool.cpp b/src/corelib/thread/qthreadpool.cpp index 51783321b9..a6c2a7e54b 100644 --- a/src/corelib/thread/qthreadpool.cpp +++ b/src/corelib/thread/qthreadpool.cpp @@ -517,6 +517,7 @@ void QThreadPool::start(QRunnable *runnable, int priority) } /*! + \fn template> void QThreadPool::start(Callable &&callableToRun, int priority) \overload \since 5.15 @@ -524,13 +525,13 @@ void QThreadPool::start(QRunnable *runnable, int priority) make the current thread count exceed maxThreadCount(). In that case, \a functionToRun is added to a run queue instead. The \a priority argument can be used to control the run queue's order of execution. + + \note This function participates in overload resolution only if \c Callable + is a function or function object which can be called with zero arguments. + + \note In Qt version prior to 6.6, this function took std::function, + and therefore couldn't handle move-only callables. */ -void QThreadPool::start(std::function functionToRun, int priority) -{ - if (!functionToRun) - return; - start(QRunnable::create(std::move(functionToRun)), priority); -} /*! Attempts to reserve a thread to run \a runnable. @@ -562,6 +563,7 @@ bool QThreadPool::tryStart(QRunnable *runnable) } /*! + \fn template> bool QThreadPool::tryStart(Callable &&callableToRun) \overload \since 5.15 Attempts to reserve a thread to run \a functionToRun. @@ -569,23 +571,13 @@ bool QThreadPool::tryStart(QRunnable *runnable) If no threads are available at the time of calling, then this function does nothing and returns \c false. Otherwise, \a functionToRun is run immediately using one available thread and this function returns \c true. + + \note This function participates in overload resolution only if \c Callable + is a function or function object which can be called with zero arguments. + + \note In Qt version prior to 6.6, this function took std::function, + and therefore couldn't handle move-only callables. */ -bool QThreadPool::tryStart(std::function functionToRun) -{ - if (!functionToRun) - return false; - - Q_D(QThreadPool); - QMutexLocker locker(&d->mutex); - if (!d->allThreads.isEmpty() && d->areAllThreadsActive()) - return false; - - QRunnable *runnable = QRunnable::create(std::move(functionToRun)); - if (d->tryStart(runnable)) - return true; - delete runnable; - return false; -} /*! \property QThreadPool::expiryTimeout \brief the thread expiry timeout value in milliseconds. @@ -799,19 +791,19 @@ void QThreadPool::startOnReservedThread(QRunnable *runnable) } /*! + \fn template> void QThreadPool::startOnReservedThread(Callable &&callableToRun) \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))); -} + \note This function participates in overload resolution only if \c Callable + is a function or function object which can be called with zero arguments. + + \note In Qt version prior to 6.6, this function took std::function, + and therefore couldn't handle move-only callables. +*/ /*! Waits up to \a msecs milliseconds for all threads to exit and removes all diff --git a/src/corelib/thread/qthreadpool.h b/src/corelib/thread/qthreadpool.h index e6d0326c4c..a097ace14b 100644 --- a/src/corelib/thread/qthreadpool.h +++ b/src/corelib/thread/qthreadpool.h @@ -36,11 +36,22 @@ public: void start(QRunnable *runnable, int priority = 0); bool tryStart(QRunnable *runnable); +#if QT_CORE_REMOVED_SINCE(6, 6) void start(std::function functionToRun, int priority = 0); bool tryStart(std::function functionToRun); +#endif void startOnReservedThread(QRunnable *runnable); +#if QT_CORE_REMOVED_SINCE(6, 6) void startOnReservedThread(std::function functionToRun); +#endif + + template = true> + void start(Callable &&functionToRun, int priority = 0); + template = true> + bool tryStart(Callable &&functionToRun); + template = true> + void startOnReservedThread(Callable &&functionToRun); int expiryTimeout() const; void setExpiryTimeout(int expiryTimeout); @@ -68,6 +79,28 @@ public: [[nodiscard]] bool tryTake(QRunnable *runnable); }; +template > +void QThreadPool::start(Callable &&functionToRun, int priority) +{ + start(QRunnable::create(std::forward(functionToRun)), priority); +} + +template > +bool QThreadPool::tryStart(Callable &&functionToRun) +{ + QRunnable *runnable = QRunnable::create(std::forward(functionToRun)); + if (tryStart(runnable)) + return true; + delete runnable; + return false; +} + +template > +void QThreadPool::startOnReservedThread(Callable &&functionToRun) +{ + startOnReservedThread(QRunnable::create(std::forward(functionToRun))); +} + QT_END_NAMESPACE #endif diff --git a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp index 42f2545574..0c4f11a0dc 100644 --- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp +++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp @@ -87,6 +87,7 @@ private slots: void takeAllAndIncreaseMaxThreadCount(); void waitForDoneAfterTake(); void threadReuse(); + void nullFunctions(); private: QMutex m_functionTestMutex; @@ -187,7 +188,7 @@ void tst_QThreadPool::runFunction3() std::unique_ptr ptr(new DeleteCheck); { TestThreadPool manager; - manager.start(QRunnable::create([my_ptr = std::move(ptr)]() { })); + manager.start([my_ptr = std::move(ptr)]() { }); } QVERIFY(DeleteCheck::s_deleted); } @@ -1465,5 +1466,22 @@ void tst_QThreadPool::threadReuse() } } +void tst_QThreadPool::nullFunctions() +{ + // Note this is not necessarily testing intended behavior, only undocumented behavior. + // If this is changed it should be noted in Behavioral Changes. + FunctionPointer nullFunction = nullptr; + std::function nullStdFunction(nullptr); + { + TestThreadPool manager; + // should not crash: + manager.start(nullFunction); + manager.start(nullStdFunction); + // should fail (and not leak): + QVERIFY(!manager.tryStart(nullStdFunction)); + QVERIFY(!manager.tryStart(nullFunction)); + } +} + QTEST_MAIN(tst_QThreadPool); #include "tst_qthreadpool.moc"