Skip to content

Commit

Permalink
ThumbnailResponse: fix a mess with switching threads upon cancelling
Browse files Browse the repository at this point in the history
This did not manifest before because the library had a bug preventing
clients from receiving BaseJob::finished() signal from an abandon()ed
job. Now the signal gets delivered, exposing the bug in ImageProvider.
  • Loading branch information
KitsuneRal committed Feb 27, 2019
1 parent 2f80263 commit 4c00203
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 13 deletions.
40 changes: 28 additions & 12 deletions client/imageprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <jobs/mediathumbnailjob.h>

#include <QtCore/QReadWriteLock>
#include <QtCore/QThread>
#include <QtCore/QDebug>

using QMatrixClient::Connection;
Expand Down Expand Up @@ -51,6 +52,7 @@ class ThumbnailResponse : public QQuickImageResponse
void startRequest()
{
// Runs in the main thread, not QML thread
Q_ASSERT(QThread::currentThread() == c->thread());
if (mediaId.count('/') != 1)
{
errorStr = QStringLiteral
Expand All @@ -60,7 +62,6 @@ class ThumbnailResponse : public QQuickImageResponse
return;
}

QWriteLocker _(&lock);
job = c->getThumbnail(mediaId, requestedSize);
// Connect to any possible outcome including abandonment
// to make sure the QML thread is not left stuck forever.
Expand All @@ -76,29 +77,43 @@ class ThumbnailResponse : public QQuickImageResponse

QImage image;
QString errorStr;
mutable QReadWriteLock lock;
mutable QReadWriteLock lock; // Guards ONLY these two above

void prepareResult()
{
// Runs in the main thread, not QML thread
Q_ASSERT(QThread::currentThread() == job->thread());
Q_ASSERT(job->error() != BaseJob::Pending);
{
QWriteLocker _(&lock);
Q_ASSERT(job->error() != BaseJob::Pending);

if (job->error() == BaseJob::Success)
{
image = job->thumbnail();
errorStr.clear();
qDebug() << "ThumbnailResponse: image ready for" << mediaId;
} else if (job->error() == BaseJob::Abandoned) {
errorStr = tr("Image request has been cancelled");
qDebug() << "ThumbnailResponse: cancelled for" << mediaId;
} else {
errorStr = job->errorString();
qWarning() << "ThumbnailResponse: no valid image for" << mediaId
<< "-" << errorStr;
}
job = nullptr;
}
job = nullptr;
emit finished();
}

void doCancel()
{
// Runs in the main thread, not QML thread
Q_ASSERT(QThread::currentThread() == job->thread());
if (job)
job->abandon();
}

// The following overrides run in QML thread

QQuickTextureFactory *textureFactory() const override
{
QReadLocker _(&lock);
Expand All @@ -113,13 +128,14 @@ class ThumbnailResponse : public QQuickImageResponse

void cancel() override
{
QWriteLocker _(&lock);
if (job)
{
job->abandon();
job = nullptr;
}
errorStr = tr("Image request has been cancelled");
// Flip from QML thread to the main thread
QMetaObject::invokeMethod(this,
#if (QT_VERSION >= QT_VERSION_CHECK(5, 10, 0))
&ThumbnailResponse::doCancel,
#else
"doCancel",
#endif
Qt::QueuedConnection);
}
};

Expand Down
2 changes: 1 addition & 1 deletion lib
Submodule lib updated from 5b5eb1 to 0975f9

0 comments on commit 4c00203

Please sign in to comment.