Merge branch 'kitsune-fix-image-provider' into 'develop'

Fix ImageProvider crashes with libQMC 0.5

See merge request b0/spectral!41

src/imageprovider.cpp

@@ -1,42 +1,37 @@
 #include "imageprovider.h"
 
-#include <connection.h>
-#include <jobs/mediathumbnailjob.h>
-
 #include <QtCore/QDebug>
-#include <QtCore/QReadWriteLock>
+#include <QtCore/QThread>
 
 using QMatrixClient::BaseJob;
-using QMatrixClient::Connection;
 
-ThumbnailResponse::ThumbnailResponse(Connection* c, QString mediaId,
-                                     const QSize& requestedSize)
+ThumbnailResponse::ThumbnailResponse(QMatrixClient::Connection* c,
+    QString mediaId, const QSize& requestedSize)
   : c(c),
     mediaId(std::move(mediaId)),
     requestedSize(requestedSize),
     errorStr("Image request hasn't started") {
-  moveToThread(c->thread());
   if (requestedSize.isEmpty()) {
     errorStr.clear();
     emit finished();
     return;
   }
+  if (mediaId.count('/') != 1) {
+    errorStr =
+        tr("Media id '%1' doesn't follow server/mediaId pattern")
+            .arg(mediaId);
+    emit finished();
+    return;
+  }
   // Execute a request on the main thread asynchronously
+  moveToThread(c->thread());
   QMetaObject::invokeMethod(this, &ThumbnailResponse::startRequest,
                             Qt::QueuedConnection);
 }
 
 void ThumbnailResponse::startRequest() {
   // Runs in the main thread, not QML thread
-  if (mediaId.count('/') != 1) {
-    errorStr =
-        QStringLiteral("Media id '%1' doesn't follow server/mediaId pattern")
-            .arg(mediaId);
-    emit finished();
-    return;
-  }
-
-  QWriteLocker _(&lock);
+  Q_ASSERT(QThread::currentThread() == c->thread());
   job = c->getThumbnail(mediaId, requestedSize);
   // Connect to any possible outcome including abandonment
   // to make sure the QML thread is not left stuck forever.
@@ -44,13 +39,16 @@ void ThumbnailResponse::startRequest() {
 }
 
 void ThumbnailResponse::prepareResult() {
+  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();
+    } 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 << "-"
@@ -61,6 +59,13 @@ void ThumbnailResponse::prepareResult() {
   emit finished();
 }
 
+void ThumbnailResponse::doCancel() {
+  // Runs in the main thread, not QML thread
+  Q_ASSERT(QThread::currentThread() == job->thread());
+  if (job)
+    job->abandon();  
+}
+
 QQuickTextureFactory* ThumbnailResponse::textureFactory() const {
   QReadLocker _(&lock);
   return QQuickTextureFactory::textureFactoryForImage(image);
@@ -72,12 +77,8 @@ QString ThumbnailResponse::errorString() const {
 }
 
 void ThumbnailResponse::cancel() {
-  QWriteLocker _(&lock);
-  if (job) {
-    job->abandon();
-    job = nullptr;
-  }
-  errorStr = "Image request has been cancelled";
+  QMetaObject::invokeMethod(this, &ThumbnailResponse::doCancel,
+                            Qt::QueuedConnection);
 }
 
 QQuickImageResponse* ImageProvider::requestImageResponse(

src/imageprovider.h

@@ -2,23 +2,29 @@
 #define IMAGEPROVIDER_H
 #pragma once
 
-#include <jobs/mediathumbnailjob.h>
-#include <QThreadPool>
-#include <QtCore/QAtomicPointer>
-#include <QtCore/QReadWriteLock>
 #include <QtQuick/QQuickAsyncImageProvider>
 
+#include <connection.h>
+#include <jobs/mediathumbnailjob.h>
+
+#include <QtCore/QReadWriteLock>
+#include <QtCore/QAtomicPointer>
+
 namespace QMatrixClient {
 class Connection;
 }
 
 class ThumbnailResponse : public QQuickImageResponse {
+  Q_OBJECT
  public:
   ThumbnailResponse(QMatrixClient::Connection* c, QString mediaId,
                     const QSize& requestedSize);
   ~ThumbnailResponse() override = default;
 
+private slots:
   void startRequest();
+  void prepareResult();
+  void doCancel();
 
  private:
   QMatrixClient::Connection* c;
@@ -28,9 +34,8 @@ class ThumbnailResponse : public QQuickImageResponse {
 
   QImage image;
   QString errorStr;
-  mutable QReadWriteLock lock;
+  mutable QReadWriteLock lock; // Guards ONLY these two members above
 
-  void prepareResult();
   QQuickTextureFactory* textureFactory() const override;
   QString errorString() const override;
   void cancel() override;
@@ -41,7 +46,7 @@ class ImageProvider : public QObject, public QQuickAsyncImageProvider {
   Q_PROPERTY(QMatrixClient::Connection* connection READ connection WRITE
                  setConnection NOTIFY connectionChanged)
  public:
-  explicit ImageProvider() : QObject(), QQuickAsyncImageProvider() {}
+  explicit ImageProvider() = default;
 
   QQuickImageResponse* requestImageResponse(
       const QString& id, const QSize& requestedSize) override;
@@ -49,6 +54,7 @@ class ImageProvider : public QObject, public QQuickAsyncImageProvider {
   QMatrixClient::Connection* connection() { return m_connection; }
   void setConnection(QMatrixClient::Connection* connection) {
     m_connection.store(connection);
+    emit connectionChanged();
   }
 
  signals: