From 64b56207f24c5aec17f9766f7bfe6424915db667 Mon Sep 17 00:00:00 2001
From: "Anthony J. Thibault" <tony@highfidelity.io>
Date: Mon, 1 Feb 2016 14:20:29 -0800
Subject: [PATCH] QmlOverlay: Fix for issue use after delete

Sometimes, it was possible to set the qmlElement bounds AFTER the qmlElement itself was deleted on another thread.
To fix this we use a shared_ptr to hold the qmlElement, then use a weak_ptr in the setProperties lambda, to verify that the qmlElement still exists.

The script that was previously causing this code to fail is: "users.js".
Because, it creates a textSizeOverlay then immediately deletes it.
---
 interface/src/ui/overlays/QmlOverlay.cpp | 28 ++++++++++++++++++------
 interface/src/ui/overlays/QmlOverlay.h   |  6 ++---
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/interface/src/ui/overlays/QmlOverlay.cpp b/interface/src/ui/overlays/QmlOverlay.cpp
index e141d38f2b..e28fd1e7ac 100644
--- a/interface/src/ui/overlays/QmlOverlay.cpp
+++ b/interface/src/ui/overlays/QmlOverlay.cpp
@@ -22,20 +22,29 @@
 #include "Application.h"
 #include "text/FontFamilies.h"
 
-QmlOverlay::QmlOverlay(const QUrl& url) { 
+QmlOverlay::QmlOverlay(const QUrl& url) {
     buildQmlElement(url);
+    qDebug() << "AJT: url = " << url << (void*)QThread::currentThreadId();
 }
 
 QmlOverlay::QmlOverlay(const QUrl& url, const QmlOverlay* textOverlay)
     : Overlay2D(textOverlay) {
+    qDebug() << "AJT: url = " << url << (void*)QThread::currentThreadId();
     buildQmlElement(url);
 }
 
 void QmlOverlay::buildQmlElement(const QUrl& url) {
     auto offscreenUi = DependencyManager::get<OffscreenUi>();
+    auto threadId = QThread::currentThreadId();
     offscreenUi->returnFromUiThread([=] {
         offscreenUi->load(url, [=](QQmlContext* context, QObject* object) {
-            _qmlElement = dynamic_cast<QQuickItem*>(object);
+            QQuickItem* rawPtr = dynamic_cast<QQuickItem*>(object);
+            // Create a shared ptr with a custom deleter lambda, that calls deleteLater
+            _qmlElement = std::shared_ptr<QQuickItem>(rawPtr, [=](QQuickItem* ptr) {
+                if (ptr) {
+                    ptr->deleteLater();
+                }
+            });
         });
         while (!_qmlElement) {
             qApp->processEvents();
@@ -54,13 +63,18 @@ QmlOverlay::~QmlOverlay() {
 void QmlOverlay::setProperties(const QScriptValue& properties) {
     Overlay2D::setProperties(properties);
     auto bounds = _bounds;
+    std::weak_ptr<QQuickItem> weakQmlElement;
     DependencyManager::get<OffscreenUi>()->executeOnUiThread([=] {
-        _qmlElement->setX(bounds.left());
-        _qmlElement->setY(bounds.top());
-        _qmlElement->setWidth(bounds.width());
-        _qmlElement->setHeight(bounds.height());
+        // check to see if qmlElement still exists
+        auto qmlElement = weakQmlElement.lock();
+        if (qmlElement) {
+            _qmlElement->setX(bounds.left());
+            _qmlElement->setY(bounds.top());
+            _qmlElement->setWidth(bounds.width());
+            _qmlElement->setHeight(bounds.height());
+        }
     });
-    QMetaObject::invokeMethod(_qmlElement, "updatePropertiesFromScript", Q_ARG(QVariant, properties.toVariant()));
+    QMetaObject::invokeMethod(_qmlElement.get(), "updatePropertiesFromScript", Q_ARG(QVariant, properties.toVariant()));
 }
 
 void QmlOverlay::render(RenderArgs* args) {
diff --git a/interface/src/ui/overlays/QmlOverlay.h b/interface/src/ui/overlays/QmlOverlay.h
index 75f1a391a3..a75783da1f 100644
--- a/interface/src/ui/overlays/QmlOverlay.h
+++ b/interface/src/ui/overlays/QmlOverlay.h
@@ -10,6 +10,7 @@
 #define hifi_QmlOverlay_h
 
 #include <QString>
+#include <memory>
 
 #include <SharedUtil.h>
 #include "Overlay2D.h"
@@ -18,7 +19,7 @@ class QQuickItem;
 
 class QmlOverlay : public Overlay2D {
     Q_OBJECT
-    
+
 public:
     QmlOverlay(const QUrl& url);
     QmlOverlay(const QUrl& url, const QmlOverlay* textOverlay);
@@ -34,8 +35,7 @@ private:
     void buildQmlElement(const QUrl& url);
 
 protected:
-    QQuickItem* _qmlElement{ nullptr };
+    std::shared_ptr<QQuickItem> _qmlElement;
 };
 
- 
 #endif // hifi_QmlOverlay_h