Rework imagescaling example to avoid potential crashes

Creating a continuation with QtFuture::Launch::Async policy does not
work well with the example, because it still needs to update the UI
once the async continuation is finished. If the user decides to
close the application while the async continuation is executed,
the next continuation will be accessing data from the destroyed
Images object.

Fix it by using QtConcurrent::run() to do the "heavy" work in a
separate thread, and use a QFutureWatcher to handle the results of
the async execution. Update the example documentation accordingly.

After this patch the example still shows the usage of continuations
and onCanceled()/onFailed() handlers. However, it now does not
illustrate the usage of different launch policies and continuation
contexts. It might not be a big issue, because the QFuture
documentation describes these topics rather extensively.

Fixes: QTBUG-103514
Pick-to: 6.5
Change-Id: I8142535064ff7a4e8007a5c0a8fe7709d6d942ec
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>

examples/qtconcurrent/imagescaling/doc/src/qtconcurrent-imagescaling.qdoc

@@ -1,4 +1,4 @@
-// Copyright (C) 2020 The Qt Company Ltd.
+// Copyright (C) 2023 The Qt Company Ltd.
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GFDL-1.3-no-invariants-only
 
 /*!
@@ -8,8 +8,9 @@
     \ingroup qtconcurrentexamples
     \brief Demonstrates how to asynchronously download and scale images.
 
-    This example shows how to use the QFuture and QPromise classes to download a
+    This example shows how to use the QFuture, QPromise, and QFutureWatcher
-    collection of images from the network and scale them, without blocking the UI.
+    classes to download a collection of images from the network and scale them,
+    without blocking the UI.
 
     \image imagescaling.webp
 
@@ -46,7 +47,6 @@
 
     And here starts the interesting part:
 
-    \dots
     \snippet imagescaling/imagescaling.cpp 11
     \dots
 
@@ -56,12 +56,14 @@
     QNetworkReply::finished() signal is emitted. This allows us to attach continuations
     and failure handlers, as it is done in the example.
 
-    In the continuation attached via \b{.then()}, we check if the user has requested to
+    In the continuation attached via \l{QFuture::then}{.then()}, we check if the
-    cancel the download. If that's the case, we stop processing the request. By calling
+    user has requested to cancel the download. If that's the case, we stop
-    the \c QPromise::finish() method, we notify the user that processing has been finished.
+    processing the request. By calling the \l QPromise::finish() method, we notify
+    the user that processing has been finished.
     In case the network request has ended with an error, we throw an exception. The
-    exception will be handled in the failure handler attached using the \b{.onFailed()}
+    exception will be handled in the failure handler attached using the
-    method. Note that we have two failure handlers: the first one captures the network
+    \l{QFuture::onFailed}{.onFailed()} method.
+    Note that we have two failure handlers: the first one captures the network
     errors, the second one all other exceptions thrown during the execution. Both handlers
     save the exception inside the promise object (to be handled by the caller of the
     \c download() method) and report that the computation has finished. Also note that,
@@ -83,7 +85,7 @@
     we need to copy and use the promise object in multiple places simultaneously. Hence,
     a QSharedPointer is used.
 
-    \c download() method is called from the \c QImage::process method. It is invoked
+    The \c download() method is called from the \c Images::process method. It is invoked
     when the user presses the \e {"Add URLs"} button:
 
     \dots
@@ -99,38 +101,20 @@
     \snippet imagescaling/imagescaling.cpp 3
     \dots
 
-    Next, we attach a continuation to handle the scaling step:
+    Next, we attach a continuation to handle the scaling step.
+    More on that later:
 
     \snippet imagescaling/imagescaling.cpp 4
     \dots
 
-    Since the scaling may be computationally heavy, and we don't want to block the main
+    After that we attach \l {QFuture::}{onCanceled()} and \l {QFuture::}{onFailed()}
-    thread, we pass the \c QtFuture::Launch::Async option, to launch the scaling step in
+    handlers:
-    a new thread. The \c scaled() method returns a list of the scaled images to the next
-    step, which takes care of showing images in the layout.
 
-    Note that \c updateStatus() is called through QMetaObject::invokeMethod(),
-    because it updates the UI and needs to be invoked from the main thread.
-
-    \dots
     \snippet imagescaling/imagescaling.cpp 5
     \dots
 
-    For the same reason \c showImages() also needs to be invoked from the main thread, so
+    The handler attached via the \l {QFuture::onCanceled}{.onCanceled()} method
-    we pass \c this as a context to \c .then(). By default, \c .then() is launched in the
+    will be called if the user has pressed the \e "Cancel" button:
-    parent's thread, but if a context object is specified, it is launched in the context
-    object's thread.
-
-    Then we add cancellation and failure handlers:
-
-    \dots
-    \snippet imagescaling/imagescaling.cpp 6
-
-    We don't need to specify the context anymore, because \c .onCanceled() and the next
-    handlers will be launched in their parent's context.
-
-    The handler attached via the \c .onCanceled() method will be called if the user has
-    pressed the \e "Cancel" button:
 
     \dots
     \snippet imagescaling/imagescaling.cpp 2
@@ -140,14 +124,55 @@
 
     \snippet imagescaling/imagescaling.cpp 7
 
-    The handlers attached via \c .onFailed() method will be called in case an
+    The handlers attached via \l {QFuture::onFailed}{.onFailed()} method will be
-    error occurred during one of the previous steps. For example, if a network error
+    called in case an error occurred during one of the previous steps.
-    has been saved inside the promise during the download step, it will be propagated to
+    For example, if a network error has been saved inside the promise during the
-    the handler that takes \c QNetworkReply::NetworkError as argument. A failure can
+    download step, it will be propagated to the handler that takes
-    happen also during the scaling step:
+    \l QNetworkReply::NetworkError as argument.
+
+    If the \c downloadFuture is not canceled, and didn't report any error, the
+    scaling continuation is executed.
+
+    Since the scaling may be computationally heavy, and we don't want to block
+    the main thread, we use \l QtConcurrent::run(), to launch the scaling step
+    in a new thread.
+
+    \snippet imagescaling/imagescaling.cpp 16
+
+    Since the scaling is launched in a separate thread, the user can potentially
+    decide to close the application while the scaling operation is in progress.
+    To handle such situations gracefully, we pass the \l QFuture returned by
+    \l QtConcurrent::run() to the \l QFutureWatcher instance.
+
+    The watcher's \l QFutureWatcher::finished signal is connected to the
+    \c Images::scaleFinished slot:
+
+    \snippet imagescaling/imagescaling.cpp 6
+
+    This slot is responsible for showing the scaled images in the UI, and also
+    for handling the errors that could potentially happen during scaling:
+
+    \snippet imagescaling/imagescaling.cpp 15
+
+    The error reporting is implemented by returning an optional from the
+    \c Images::scaled() method:
 
     \snippet imagescaling/imagescaling.cpp 14
 
+    The \c Images::OptionalImages type here is simply a typedef for \c std::optional:
+
+    \snippet imagescaling/imagescaling.h 1
+
+    \note We cannot handle the errors from the async scaling operation using
+    the \l {QFuture::onFailed}{.onFailed()} handler, because the handler needs
+    to be executed in the context of \c Images object in the UI thread.
+    If the user closes the application while the async computation is done,
+    the \c Images object will be destroyed, and accessing its members from the
+    continuation will lead to a crash. Using \l QFutureWatcher and its signals
+    allows us to avoid the problem, because the signals are disconnected when
+    the \l QFutureWatcher is destroyed, so the related slots will never be
+    executed in a destroyed context.
+
     The rest of the code is straightforward, you can check the example project for
     more details.
 

examples/qtconcurrent/imagescaling/imagescaling.cpp

@@ -1,12 +1,10 @@
-// Copyright (C) 2020 The Qt Company Ltd.
+// Copyright (C) 2023 The Qt Company Ltd.
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
+
 #include "imagescaling.h"
 #include "downloaddialog.h"
 
 #include <QNetworkReply>
-#include <QtMath>
-
-#include <functional>
 
 Images::Images(QWidget *parent) : QWidget(parent), downloadDialog(new DownloadDialog(this))
 {
@@ -38,6 +36,11 @@ Images::Images(QWidget *parent) : QWidget(parent), downloadDialog(new DownloadDi
     mainLayout->addStretch();
     mainLayout->addWidget(statusBar);
     setLayout(mainLayout);
+
+//! [6]
+    connect(&scalingWatcher, &QFutureWatcher<QList<QImage>>::finished,
+            this, &Images::scaleFinished);
+//! [6]
 }
 
 Images::~Images()
@@ -50,6 +53,7 @@ void Images::process()
 {
     // Clean previous state
     replies.clear();
+    addUrlsButton->setEnabled(false);
 
     if (downloadDialog->exec() == QDialog::Accepted) {
 
@@ -65,33 +69,34 @@ void Images::process()
         statusBar->showMessage(tr("Downloading..."));
 //! [3]
 
-//! [4]
+        //! [4]
-        downloadFuture.then([this](auto) { cancelButton->setEnabled(false); })
+        downloadFuture
-                .then(QtFuture::Launch::Async,
+                .then([this](auto) {
-                      [this] {
+                    cancelButton->setEnabled(false);
-                          QMetaObject::invokeMethod(this,
+                    updateStatus(tr("Scaling..."));
-                                                    [this] { updateStatus(tr("Scaling...")); });
+                    //! [16]
-                          return scaled();
+                    scalingWatcher.setFuture(QtConcurrent::run(Images::scaled,
-                      })
+                                                               downloadFuture.results()));
-//! [4]
+                    //! [16]
-//! [5]
+                })
-                .then(this, [this](const QList<QImage> &scaled) {
+        //! [4]
-                    showImages(scaled);
+        //! [5]
-                    updateStatus(tr("Finished"));
+                .onCanceled([this] {
+                    updateStatus(tr("Download has been canceled."));
                 })
-//! [5]
-//! [6]
-                .onCanceled([this] { updateStatus(tr("Download has been canceled.")); })
                 .onFailed([this](QNetworkReply::NetworkError error) {
                     updateStatus(tr("Download finished with error: %1").arg(error));
-
                     // Abort all pending requests
                     abortDownload();
                 })
                 .onFailed([this](const std::exception &ex) {
                     updateStatus(tr(ex.what()));
+                })
+        //! [5]
+                .then([this]() {
+                    cancelButton->setEnabled(false);
+                    addUrlsButton->setEnabled(true);
                 });
-//! [6]
     }
 }
 
@@ -105,22 +110,37 @@ void Images::cancel()
 }
 //! [7]
 
+//! [15]
+void Images::scaleFinished()
+{
+    const OptionalImages result = scalingWatcher.result();
+    if (result.has_value()) {
+        const auto scaled = result.value();
+        showImages(scaled);
+        updateStatus(tr("Finished"));
+    } else {
+        updateStatus(tr("Failed to extract image data."));
+    }
+    addUrlsButton->setEnabled(true);
+}
+//! [15]
+
 //! [8]
 QFuture<QByteArray> Images::download(const QList<QUrl> &urls)
-//! [8]
 {
+//! [8]
 //! [9]
     QSharedPointer<QPromise<QByteArray>> promise(new QPromise<QByteArray>());
     promise->start();
 //! [9]
 
-//! [10]
+    //! [10]
     for (const auto &url : urls) {
         QSharedPointer<QNetworkReply> reply(qnam.get(QNetworkRequest(url)));
         replies.push_back(reply);
-//! [10]
+    //! [10]
 
-//! [11]
+    //! [11]
         QtFuture::connect(reply.get(), &QNetworkReply::finished).then([=] {
             if (promise->isCanceled()) {
                 if (!promise->future().isFinished())
@@ -132,14 +152,13 @@ QFuture<QByteArray> Images::download(const QList<QUrl> &urls)
                 if (!promise->future().isFinished())
                     throw reply->error();
             }
-//! [12]
+        //! [12]
             promise->addResult(reply->readAll());
 
             // Report finished on the last download
-            if (promise->future().resultCount() == urls.size()) {
+            if (promise->future().resultCount() == urls.size())
                 promise->finish();
-            }
+        //! [12]
-//! [12]
         }).onFailed([promise] (QNetworkReply::NetworkError error) {
             promise->setException(std::make_exception_ptr(error));
             promise->finish();
@@ -150,7 +169,7 @@ QFuture<QByteArray> Images::download(const QList<QUrl> &urls)
             promise->finish();
         });
     }
-//! [11]
+    //! [11]
 
 //! [13]
     return promise->future();
@@ -158,15 +177,14 @@ QFuture<QByteArray> Images::download(const QList<QUrl> &urls)
 //! [13]
 
 //! [14]
-QList<QImage> Images::scaled() const
+Images::OptionalImages Images::scaled(const QList<QByteArray> &data)
 {
     QList<QImage> scaled;
-    const auto data = downloadFuture.results();
     for (const auto &imgData : data) {
         QImage image;
         image.loadFromData(imgData);
         if (image.isNull())
-            throw std::runtime_error("Failed to load image.");
+            return std::nullopt;
 
         scaled.push_back(image.scaled(100, 100, Qt::KeepAspectRatio));
     }

examples/qtconcurrent/imagescaling/imagescaling.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2020 The Qt Company Ltd.
+// Copyright (C) 2023 The Qt Company Ltd.
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
 #ifndef IMAGESCALING_H
 #define IMAGESCALING_H
@@ -6,6 +6,7 @@
 #include <QtWidgets>
 #include <QtConcurrent>
 #include <QNetworkAccessManager>
+#include <optional>
 
 class DownloadDialog;
 class Images : public QWidget
@@ -18,7 +19,6 @@ public:
     void initLayout(qsizetype count);
 
     QFuture<QByteArray> download(const QList<QUrl> &urls);
-    QList<QImage> scaled() const;
     void updateStatus(const QString &msg);
     void showImages(const QList<QImage> &images);
     void abortDownload();
@@ -27,7 +27,15 @@ public slots:
     void process();
     void cancel();
 
+private slots:
+    void scaleFinished();
+
 private:
+    //! [1]
+    using OptionalImages = std::optional<QList<QImage>>;
+    //! [1]
+    static OptionalImages scaled(const QList<QByteArray> &data);
+
     QPushButton *addUrlsButton;
     QPushButton *cancelButton;
     QVBoxLayout *mainLayout;
@@ -39,6 +47,7 @@ private:
     QNetworkAccessManager qnam;
     QList<QSharedPointer<QNetworkReply>> replies;
     QFuture<QByteArray> downloadFuture;
+    QFutureWatcher<OptionalImages> scalingWatcher;
 };
 
 #endif // IMAGESCALING_H

src/concurrent/doc/qtconcurrent.qdocconf

@@ -26,7 +26,7 @@ qhp.QtConcurrent.subprojects.examples.selectors = fake:example
 
 tagfile                 = ../../../doc/qtconcurrent/qtconcurrent.tags
 
-depends                 += qtcore qtdoc qmake qtcmake
+depends                 += qtcore qtnetwork qtdoc qmake qtcmake
 
 headerdirs  += ..