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.
This commit is contained in:
Anthony J. Thibault 2016-02-01 14:20:29 -08:00
parent b553c8400d
commit 64b56207f2
2 changed files with 24 additions and 10 deletions

View file

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

View file

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