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 <marc.mutz@qt.io>

src/corelib/compat/removed_api.cpp

@@ -524,6 +524,45 @@ qsizetype QString::toUcs4_helper(const ushort *uc, qsizetype length, uint *out)
                          reinterpret_cast<char32_t *>(out));
 }
 
+#if QT_CONFIG(thread)
+
+#include "qthreadpool.h"
+#include "private/qthreadpool_p.h"
+
+void QThreadPool::start(std::function<void()> functionToRun, int priority)
+{
+    if (!functionToRun)
+        return;
+    start(QRunnable::create(std::move(functionToRun)), priority);
+}
+
+bool QThreadPool::tryStart(std::function<void()> 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<void()> 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

src/corelib/thread/qrunnable.h

@@ -4,7 +4,9 @@
 #ifndef QRUNNABLE_H
 #define QRUNNABLE_H
 
-#include <QtCore/qglobal.h>
+#include <QtCore/qcompilerdetection.h>
+#include <QtCore/qdebug.h>
+
 #include <functional>
 #include <type_traits>
 
@@ -28,6 +30,8 @@ public:
 
     template <typename Callable, if_callable<Callable> = 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 <typename UniCallable>
         QGenericRunnableHelper(UniCallable &&functionToRun) noexcept :
               QGenericRunnableHelperBase(
-                      [](const QGenericRunnableHelperBase *that) { static_cast<const QGenericRunnableHelper*>(that)->m_functionToRun(); },
-                      [](const QGenericRunnableHelperBase *that) { delete static_cast<const QGenericRunnableHelper*>(that); }),
+                      [](QGenericRunnableHelperBase *that) { static_cast<QGenericRunnableHelper*>(that)->m_functionToRun(); },
+                      [](QGenericRunnableHelperBase *that) { delete static_cast<QGenericRunnableHelper*>(that); }),
               m_functionToRun(std::forward<UniCallable>(functionToRun))
         {
         }
@@ -79,9 +83,33 @@ public:
     }
 };
 
+namespace QtPrivate {
+
+template <typename T>
+using is_function_pointer = std::conjunction<std::is_pointer<T>, std::is_function<std::remove_pointer_t<T>>>;
+template <typename T>
+struct is_std_function : std::false_type {};
+template <typename T>
+struct is_std_function<std::function<T>> : std::true_type {};
+
+} // namespace QtPrivate
+
 template <typename Callable, QRunnable::if_callable<Callable>>
 QRunnable *QRunnable::create(Callable &&functionToRun)
 {
+    bool is_null = false;
+    if constexpr(QtPrivate::is_std_function<std::decay_t<Callable>>::value)
+        is_null = !functionToRun;
+
+    if constexpr(QtPrivate::is_function_pointer<std::decay_t<Callable>>::value) {
+        const void *functionPtr = reinterpret_cast<void *>(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::decay_t<Callable>>(
                     std::forward<Callable>(functionToRun)));

src/corelib/thread/qthreadpool.cpp

@@ -517,6 +517,7 @@ void QThreadPool::start(QRunnable *runnable, int priority)
 }
 
 /*!
+    \fn template<typename Callable, QRunnable::if_callable<Callable>> 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<void()>,
+    and therefore couldn't handle move-only callables.
 */
-void QThreadPool::start(std::function<void()> 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<typename Callable, QRunnable::if_callable<Callable>> 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<void()>,
+    and therefore couldn't handle move-only callables.
 */
-bool QThreadPool::tryStart(std::function<void()> 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<typename Callable, QRunnable::if_callable<Callable>> 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<void()> 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<void()>,
+    and therefore couldn't handle move-only callables.
+*/
 
 /*!
     Waits up to \a msecs milliseconds for all threads to exit and removes all

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<void()> functionToRun, int priority = 0);
     bool tryStart(std::function<void()> functionToRun);
+#endif
 
     void startOnReservedThread(QRunnable *runnable);
+#if QT_CORE_REMOVED_SINCE(6, 6)
     void startOnReservedThread(std::function<void()> functionToRun);
+#endif
+
+    template <typename Callable, QRunnable::if_callable<Callable> = true>
+    void start(Callable &&functionToRun, int priority = 0);
+    template <typename Callable, QRunnable::if_callable<Callable> = true>
+    bool tryStart(Callable &&functionToRun);
+    template <typename Callable, QRunnable::if_callable<Callable> = true>
+    void startOnReservedThread(Callable &&functionToRun);
 
     int expiryTimeout() const;
     void setExpiryTimeout(int expiryTimeout);
@@ -68,6 +79,28 @@ public:
     [[nodiscard]] bool tryTake(QRunnable *runnable);
 };
 
+template <typename Callable, QRunnable::if_callable<Callable>>
+void QThreadPool::start(Callable &&functionToRun, int priority)
+{
+    start(QRunnable::create(std::forward<Callable>(functionToRun)), priority);
+}
+
+template <typename Callable, QRunnable::if_callable<Callable>>
+bool QThreadPool::tryStart(Callable &&functionToRun)
+{
+    QRunnable *runnable = QRunnable::create(std::forward<Callable>(functionToRun));
+    if (tryStart(runnable))
+        return true;
+    delete runnable;
+    return false;
+}
+
+template <typename Callable, QRunnable::if_callable<Callable>>
+void QThreadPool::startOnReservedThread(Callable &&functionToRun)
+{
+    startOnReservedThread(QRunnable::create(std::forward<Callable>(functionToRun)));
+}
+
 QT_END_NAMESPACE
 
 #endif

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<DeleteCheck> 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<void()> 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"