Fix QFuture continuations/handlers to work with move-only callables

std::function, which is used to store the type-erased continuation
lambdas, requires the passed callable to be copy-constructible. This
makes impossible to use move-only callables with continuations/handlers.
In particular, it makes impossible passing lambdas that are capturing
move-only objects. The workaround is to store the continuation lambda
inside a wrapper for the callable, which stores the move-only lambda in
a QSharedPtr and can be stored in std::function, since it's copyable.

Pick-to: 6.2
Fixes: QTBUG-98493
Change-Id: I8b7a22fcf68dc132b3c533216a7a1665e9f9fb0a
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/corelib/thread/qfuture_impl.h

@@ -498,6 +498,17 @@ bool Continuation<Function, ResultType, ParentResultType>::execute()
     return true;
 }
 
+// Workaround for keeping move-only lambdas inside std::function
+template<class Function>
+struct ContinuationWrapper
+{
+    ContinuationWrapper(Function &&f) : function(QSharedPointer<Function>::create(std::move(f))) { }
+    void operator()(const QFutureInterfaceBase &parentData) { (*function)(parentData); }
+
+private:
+    QSharedPointer<Function> function;
+};
+
 template<typename Function, typename ResultType, typename ParentResultType>
 template<typename F>
 void Continuation<Function, ResultType, ParentResultType>::create(F &&func,
@@ -543,8 +554,10 @@ void Continuation<Function, ResultType, ParentResultType>::create(F &&func,
             continuationJob = nullptr;
         }
     };
-
+    if constexpr (!std::is_copy_constructible_v<Function>)
-    f->d.setContinuation(std::move(continuation), p.d);
+        f->d.setContinuation(ContinuationWrapper(std::move(continuation)), p.d);
+    else
+        f->d.setContinuation(std::move(continuation), p.d);
 }
 
 template<typename Function, typename ResultType, typename ParentResultType>
@@ -573,7 +586,10 @@ void Continuation<Function, ResultType, ParentResultType>::create(F &&func,
         }
     };
 
-    f->d.setContinuation(std::move(continuation), p.d);
+    if constexpr (!std::is_copy_constructible_v<Function>)
+        f->d.setContinuation(ContinuationWrapper(std::move(continuation)), p.d);
+    else
+        f->d.setContinuation(std::move(continuation), p.d);
 }
 
 template<typename Function, typename ResultType, typename ParentResultType>
@@ -596,7 +612,10 @@ void Continuation<Function, ResultType, ParentResultType>::create(F &&func,
         });
     };
 
-    f->d.setContinuation(std::move(continuation), p.d);
+    if constexpr (!std::is_copy_constructible_v<Function>)
+        f->d.setContinuation(ContinuationWrapper(std::move(continuation)), p.d);
+    else
+        f->d.setContinuation(std::move(continuation), p.d);
 }
 
 template<typename Function, typename ResultType, typename ParentResultType>
@@ -675,7 +694,10 @@ void FailureHandler<Function, ResultType>::create(F &&function, QFuture<ResultTy
         failureHandler.run();
     };
 
-    future->d.setContinuation(std::move(failureContinuation));
+    if constexpr (!std::is_copy_constructible_v<Function>)
+        future->d.setContinuation(ContinuationWrapper(std::move(failureContinuation)));
+    else
+        future->d.setContinuation(std::move(failureContinuation));
 }
 
 template<class Function, class ResultType>
@@ -699,7 +721,10 @@ void FailureHandler<Function, ResultType>::create(F &&function, QFuture<ResultTy
                         });
             };
 
-    future->d.setContinuation(std::move(failureContinuation));
+    if constexpr (!std::is_copy_constructible_v<Function>)
+        future->d.setContinuation(ContinuationWrapper(std::move(failureContinuation)));
+    else
+        future->d.setContinuation(std::move(failureContinuation));
 }
 
 template<class Function, class ResultType>
@@ -782,7 +807,12 @@ public:
             auto parentFuture = QFutureInterface<ResultType>(parentData).future();
             run(std::forward<F>(handler), parentFuture, promise);
         };
-        future->d.setContinuation(std::move(canceledContinuation));
+
+        if constexpr (!std::is_copy_constructible_v<Function>)
+            future->d.setContinuation(ContinuationWrapper(std::move(canceledContinuation)));
+        else
+            future->d.setContinuation(std::move(canceledContinuation));
+
         return promise.future();
     }
 
@@ -802,7 +832,12 @@ public:
                         run(std::forward<F>(handler), parentFuture, promise);
                     });
         };
-        future->d.setContinuation(std::move(canceledContinuation));
+
+        if constexpr (!std::is_copy_constructible_v<Function>)
+            future->d.setContinuation(ContinuationWrapper(std::move(canceledContinuation)));
+        else
+            future->d.setContinuation(std::move(canceledContinuation));
+
         return promise.future();
     }
 

tests/auto/corelib/thread/qfuture/CMakeLists.txt

@@ -12,3 +12,8 @@ qt_internal_add_test(tst_qfuture
     PUBLIC_LIBRARIES
         Qt::CorePrivate
 )
+
+qt_internal_extend_target(tst_qmetatype CONDITION MSVC
+    COMPILE_OPTIONS
+        /bigobj
+)

tests/auto/corelib/thread/qfuture/tst_qfuture.cpp

@@ -145,6 +145,7 @@ private slots:
     void onCanceled();
     void cancelContinuations();
     void continuationsWithContext();
+    void continuationsWithMoveOnlyLambda();
 #if 0
     // TODO: enable when QFuture::takeResults() is enabled
     void takeResults();
@@ -3111,6 +3112,71 @@ void tst_QFuture::continuationsWithContext()
     thread.wait();
 }
 
+void tst_QFuture::continuationsWithMoveOnlyLambda()
+{
+    // .then()
+    {
+        std::unique_ptr<int> uniquePtr(new int(42));
+        auto future = QtFuture::makeReadyFuture().then([p = std::move(uniquePtr)] { return *p; });
+        QCOMPARE(future.result(), 42);
+    }
+    // .then() with thread pool
+    {
+        QThreadPool pool;
+
+        std::unique_ptr<int> uniquePtr(new int(42));
+        auto future =
+                QtFuture::makeReadyFuture().then(&pool, [p = std::move(uniquePtr)] { return *p; });
+        QCOMPARE(future.result(), 42);
+    }
+    // .then() with context
+    {
+        QObject object;
+
+        std::unique_ptr<int> uniquePtr(new int(42));
+        auto future = QtFuture::makeReadyFuture().then(&object,
+                                                       [p = std::move(uniquePtr)] { return *p; });
+        QCOMPARE(future.result(), 42);
+    }
+
+    // .onCanceled()
+    {
+        std::unique_ptr<int> uniquePtr(new int(42));
+        auto future =
+                createCanceledFuture<int>().onCanceled([p = std::move(uniquePtr)] { return *p; });
+        QCOMPARE(future.result(), 42);
+    }
+
+    // .onCanceled() with context
+    {
+        QObject object;
+
+        std::unique_ptr<int> uniquePtr(new int(42));
+        auto future = createCanceledFuture<int>().onCanceled(
+                &object, [p = std::move(uniquePtr)] { return *p; });
+        QCOMPARE(future.result(), 42);
+    }
+
+#ifndef QT_NO_EXCEPTIONS
+    // .onFailed()
+    {
+        std::unique_ptr<int> uniquePtr(new int(42));
+        auto future = QtFuture::makeExceptionalFuture<int>(QException())
+                              .onFailed([p = std::move(uniquePtr)] { return *p; });
+        QCOMPARE(future.result(), 42);
+    }
+    // .onFailed() with context
+    {
+        QObject object;
+
+        std::unique_ptr<int> uniquePtr(new int(42));
+        auto future = QtFuture::makeExceptionalFuture<int>(QException())
+                              .onFailed(&object, [p = std::move(uniquePtr)] { return *p; });
+        QCOMPARE(future.result(), 42);
+    }
+#endif // QT_NO_EXCEPTIONS
+}
+
 void tst_QFuture::testSingleResult(const UniquePtr &p)
 {
     QVERIFY(p.get() != nullptr);