1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Rework imagescaling example to avoid potential crashes

Browse Source 
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
Change-Id: I8142535064ff7a4e8007a5c0a8fe7709d6d942ec
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
(cherry picked from commit 5ddb5d1fee52700b71701203ac622b3b363eab46)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Ivan Solovev 2023-02-17 10:19:14 +01:00 committed by Qt Cherry-pick Bot
parent e5fb960796
commit 955377a6c3
4 changed files with 128 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
examples/qtconcurrent/imagescaling/doc/src/qtconcurrent-imagescaling.qdoc
View File

@ -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
collection of images from the network and scale them, without blocking the UI.
This example shows how to use the QFuture, QPromise, and QFutureWatcher
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
cancel the download. If that's the case, we stop processing the request. By calling
the \c QPromise::finish() method, we notify the user that processing has been finished.
In the continuation attached via \l{QFuture::then}{.then()}, we check if the
user has requested to cancel the download. If that's the case, we stop
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()}
method. Note that we have two failure handlers: the first one captures the network
exception will be handled in the failure handler attached using the
\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
thread, we pass the \c QtFuture::Launch::Async option, to launch the scaling step in
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.
After that we attach \l {QFuture::}{onCanceled()} and \l {QFuture::}{onFailed()}
handlers:
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
we pass \c this as a context to \c .then(). By default, \c .then() is launched in the
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:
The handler attached via the \l {QFuture::onCanceled}{.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
error occurred during one of the previous steps. For example, if a network error
has been saved inside the promise during the download step, it will be propagated to
the handler that takes \c QNetworkReply::NetworkError as argument. A failure can
happen also during the scaling step:
The handlers attached via \l {QFuture::onFailed}{.onFailed()} method will be
called in case an error occurred during one of the previous steps.
For example, if a network error has been saved inside the promise during the
download step, it will be propagated to the handler that takes
\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.

86
examples/qtconcurrent/imagescaling/imagescaling.cpp
View File

@ -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]
downloadFuture.then([this](auto) { cancelButton->setEnabled(false); })
.then(QtFuture::Launch::Async,
[this] {
QMetaObject::invokeMethod(this,
[this] { updateStatus(tr("Scaling...")); });
return scaled();
})
//! [4]
//! [5]
.then(this, [this](const QList<QImage> &scaled) {
showImages(scaled);
updateStatus(tr("Finished"));
//! [4]
downloadFuture
.then([this](auto) {
cancelButton->setEnabled(false);
updateStatus(tr("Scaling..."));
//! [16]
scalingWatcher.setFuture(QtConcurrent::run(Images::scaled,
downloadFuture.results()));
//! [16]
})
//! [4]
//! [5]
.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));
}

13
examples/qtconcurrent/imagescaling/imagescaling.h
View File

@ -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

2
src/concurrent/doc/qtconcurrent.qdocconf
View File

@ -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 += ..