From 79e528633584f111a2c9a9fcdec07e8b798838d0 Mon Sep 17 00:00:00 2001
From: Zach Pomerantz <zmp@umich.edu>
Date: Sun, 3 Apr 2016 11:53:59 -0700
Subject: [PATCH] Fix ImageReader threading issues

---
 .../src/model-networking/TextureCache.cpp     | 58 ++++++++++++-------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp
index 3ba36dc2da..ff189b46f7 100644
--- a/libraries/model-networking/src/model-networking/TextureCache.cpp
+++ b/libraries/model-networking/src/model-networking/TextureCache.cpp
@@ -196,7 +196,7 @@ NetworkTexture::NetworkTexture(const QUrl& url, const TextureLoaderFunc& texture
 {
     _textureLoader = textureLoader;
 }
-    
+
 NetworkTexture::TextureLoaderFunc NetworkTexture::getTextureLoader() const {
     switch (_type) {
         case CUBE_TEXTURE: {
@@ -240,14 +240,14 @@ NetworkTexture::TextureLoaderFunc NetworkTexture::getTextureLoader() const {
 class ImageReader : public QRunnable {
 public:
 
-    ImageReader(const QWeakPointer<Resource>& texture, const QByteArray& data, const QUrl& url = QUrl());
+    ImageReader(const QWeakPointer<Resource>& resource, const QByteArray& data, const QUrl& url = QUrl());
 
     virtual void run();
 
 private:
     static void listSupportedImageFormats();
 
-    QWeakPointer<Resource> _texture;
+    QWeakPointer<Resource> _resource;
     QUrl _url;
     QByteArray _content;
 };
@@ -261,9 +261,9 @@ void NetworkTexture::loadContent(const QByteArray& content) {
     QThreadPool::globalInstance()->start(new ImageReader(_self, content, _url));
 }
 
-ImageReader::ImageReader(const QWeakPointer<Resource>& texture, const QByteArray& data,
+ImageReader::ImageReader(const QWeakPointer<Resource>& resource, const QByteArray& data,
         const QUrl& url) :
-    _texture(texture),
+    _resource(resource),
     _url(url),
     _content(data)
 {
@@ -284,25 +284,25 @@ void ImageReader::run() {
     }
     QThread::currentThread()->setPriority(QThread::LowPriority);
 
-    auto texture = _texture.toStrongRef();
-    if (!texture) {
-        qCWarning(modelnetworking) << "Could not get strong ref";
+    if (!_resource.data()) {
+        qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
+        QThread::currentThread()->setPriority(originalPriority);
         return;
     }
 
     listSupportedImageFormats();
 
-    // try to help the QImage loader by extracting the image file format from the url filename ext
-    // Some tga are not created properly for example without it
+    // Help the QImage loader by extracting the image file format from the url filename ext.
+    // Some tga are not created properly without it.
     auto filename = _url.fileName().toStdString();
     auto filenameExtension = filename.substr(filename.find_last_of('.') + 1);
     QImage image = QImage::fromData(_content, filenameExtension.c_str());
 
     // Note that QImage.format is the pixel format which is different from the "format" of the image file...
-    auto imageFormat = image.format(); 
+    auto imageFormat = image.format();
     int originalWidth = image.width();
     int originalHeight = image.height();
-    
+
     if (originalWidth == 0 || originalHeight == 0 || imageFormat == QImage::Format_Invalid) {
         if (filenameExtension.empty()) {
             qCDebug(modelnetworking) << "QImage failed to create from content, no file extension:" << _url;
@@ -312,15 +312,31 @@ void ImageReader::run() {
         return;
     }
 
-    gpu::Texture* theTexture = nullptr;
-    auto ntex = texture.dynamicCast<NetworkTexture>();
-    if (ntex) {
-        theTexture = ntex->getTextureLoader()(image, _url.toString().toStdString());
+    gpu::Texture* texture = nullptr;
+    {
+        // Double-check the resource still exists between long operations.
+        auto resource = _resource.toStrongRef();
+        if (!resource) {
+            qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
+            QThread::currentThread()->setPriority(originalPriority);
+            return;
+        }
+
+        auto url = _url.toString().toStdString();
+        texture = resource.dynamicCast<NetworkTexture>()->getTextureLoader()(image, url);
+    }
+
+    // Ensure the resource has not been deleted, and won't be while invokeMethod is in flight.
+    auto resource = _resource.toStrongRef();
+    if (!resource) {
+        qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
+        delete texture;
+    } else {
+        QMetaObject::invokeMethod(resource.data(), "setImage", Qt::BlockingQueuedConnection,
+            Q_ARG(void*, texture),
+            Q_ARG(int, originalWidth), Q_ARG(int, originalHeight));
     }
 
-    QMetaObject::invokeMethod(texture.data(), "setImage", 
-        Q_ARG(void*, theTexture),
-        Q_ARG(int, originalWidth), Q_ARG(int, originalHeight));
     QThread::currentThread()->setPriority(originalPriority);
 }
 
@@ -328,9 +344,9 @@ void NetworkTexture::setImage(void* voidTexture, int originalWidth,
                               int originalHeight) {
     _originalWidth = originalWidth;
     _originalHeight = originalHeight;
-    
+
     gpu::Texture* texture = static_cast<gpu::Texture*>(voidTexture);
-    
+
     // Passing ownership
     _textureSource->resetTexture(texture);
     auto gpuTexture = _textureSource->getGPUTexture();