From eb20181b6086c487542ac56e1773da8ef0d19d91 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 27 Mar 2018 14:18:01 -0700 Subject: [PATCH 1/4] Shut down the overlay interface on exit --- interface/src/ui/overlays/Overlays.cpp | 34 +++++++++++++++++++++++++- interface/src/ui/overlays/Overlays.h | 1 + 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/interface/src/ui/overlays/Overlays.cpp b/interface/src/ui/overlays/Overlays.cpp index 35274e4fbe..ee2f62c6bb 100644 --- a/interface/src/ui/overlays/Overlays.cpp +++ b/interface/src/ui/overlays/Overlays.cpp @@ -54,6 +54,7 @@ Overlays::Overlays() { } void Overlays::cleanupAllOverlays() { + _shuttingDown = true; QMap overlaysHUD; QMap overlaysWorld; { @@ -147,6 +148,10 @@ void Overlays::enable() { // Note, can't be invoked by scripts, but can be called by the InterfaceParentFinder // class on packet processing threads Overlay::Pointer Overlays::getOverlay(OverlayID id) const { + if (_shuttingDown) { + return nullptr; + } + QMutexLocker locker(&_mutex); if (_overlaysHUD.contains(id)) { return _overlaysHUD[id]; @@ -157,6 +162,10 @@ Overlay::Pointer Overlays::getOverlay(OverlayID id) const { } OverlayID Overlays::addOverlay(const QString& type, const QVariant& properties) { + if (_shuttingDown) { + return UNKNOWN_OVERLAY_ID; + } + if (QThread::currentThread() != thread()) { OverlayID result; PROFILE_RANGE(script, __FUNCTION__); @@ -261,6 +270,10 @@ OverlayID Overlays::addOverlay(const QString& type, const QVariant& properties) } OverlayID Overlays::addOverlay(const Overlay::Pointer& overlay) { + if (_shuttingDown) { + return UNKNOWN_OVERLAY_ID; + } + OverlayID thisID = OverlayID(QUuid::createUuid()); overlay->setOverlayID(thisID); overlay->setStackOrder(_stackOrder++); @@ -283,6 +296,10 @@ OverlayID Overlays::addOverlay(const Overlay::Pointer& overlay) { } OverlayID Overlays::cloneOverlay(OverlayID id) { + if (_shuttingDown) { + return UNKNOWN_OVERLAY_ID; + } + if (QThread::currentThread() != thread()) { OverlayID result; PROFILE_RANGE(script, __FUNCTION__); @@ -301,6 +318,10 @@ OverlayID Overlays::cloneOverlay(OverlayID id) { } bool Overlays::editOverlay(OverlayID id, const QVariant& properties) { + if (_shuttingDown) { + return false; + } + auto thisOverlay = getOverlay(id); if (!thisOverlay) { return false; @@ -320,6 +341,10 @@ bool Overlays::editOverlay(OverlayID id, const QVariant& properties) { } bool Overlays::editOverlays(const QVariant& propertiesById) { + if (_shuttingDown) { + return false; + } + bool defer2DOverlays = QThread::currentThread() != thread(); QVariantMap deferrred; @@ -351,6 +376,10 @@ bool Overlays::editOverlays(const QVariant& propertiesById) { } void Overlays::deleteOverlay(OverlayID id) { + if (_shuttingDown) { + return; + } + if (QThread::currentThread() != thread()) { QMetaObject::invokeMethod(this, "deleteOverlay", Q_ARG(OverlayID, id)); return; @@ -374,6 +403,9 @@ void Overlays::deleteOverlay(OverlayID id) { } QString Overlays::getOverlayType(OverlayID overlayId) { + if (_shuttingDown) { + return ""; + } if (QThread::currentThread() != thread()) { QString result; PROFILE_RANGE(script, __FUNCTION__); @@ -389,7 +421,7 @@ QString Overlays::getOverlayType(OverlayID overlayId) { } OverlayID Overlays::getOverlayAtPoint(const glm::vec2& point) { - if (!_enabled) { + if (_shuttingDown || !_enabled) { return UNKNOWN_OVERLAY_ID; } diff --git a/interface/src/ui/overlays/Overlays.h b/interface/src/ui/overlays/Overlays.h index c3d87642f1..8eccbfa4c5 100644 --- a/interface/src/ui/overlays/Overlays.h +++ b/interface/src/ui/overlays/Overlays.h @@ -680,6 +680,7 @@ private: unsigned int _stackOrder { 1 }; bool _enabled = true; + std::atomic _shuttingDown{ false }; PointerEvent calculateOverlayPointerEvent(OverlayID overlayID, PickRay ray, RayToOverlayIntersectionResult rayPickResult, QMouseEvent* event, PointerEvent::EventType eventType); From 29106fbbbe422527fed7b5f10a6f57bea2528124 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 27 Mar 2018 14:18:19 -0700 Subject: [PATCH 2/4] Prevent potential double delete of QML overlays --- interface/src/ui/overlays/QmlOverlay.cpp | 32 ++++++++++++------------ interface/src/ui/overlays/QmlOverlay.h | 3 ++- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/interface/src/ui/overlays/QmlOverlay.cpp b/interface/src/ui/overlays/QmlOverlay.cpp index f4a9034187..2a583e0450 100644 --- a/interface/src/ui/overlays/QmlOverlay.cpp +++ b/interface/src/ui/overlays/QmlOverlay.cpp @@ -39,18 +39,20 @@ void QmlOverlay::buildQmlElement(const QUrl& url) { auto offscreenUi = DependencyManager::get(); offscreenUi->load(url, [=](QQmlContext* context, QObject* object) { - QQuickItem* rawPtr = dynamic_cast(object); - // Create a shared ptr with a custom deleter lambda, that calls deleteLater - _qmlElement = std::shared_ptr(rawPtr, [](QQuickItem* ptr) { - if (ptr) { - ptr->deleteLater(); - } - }); + _qmlElement = dynamic_cast(object); + connect(_qmlElement, &QObject::destroyed, this, &QmlOverlay::qmlElementDestroyed); }); } +void QmlOverlay::qmlElementDestroyed() { + _qmlElement = nullptr; +} + QmlOverlay::~QmlOverlay() { - _qmlElement.reset(); + if (_qmlElement) { + _qmlElement->deleteLater(); + } + _qmlElement = nullptr; } // QmlOverlay replaces Overlay's properties with those defined in the QML file used but keeps Overlay2D's properties. @@ -62,15 +64,13 @@ void QmlOverlay::setProperties(const QVariantMap& properties) { Overlay2D::setProperties(properties); auto bounds = _bounds; - std::weak_ptr weakQmlElement = _qmlElement; // 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.get(), "updatePropertiesFromScript", Qt::DirectConnection, Q_ARG(QVariant, properties)); + if (_qmlElement) { + _qmlElement->setX(bounds.left()); + _qmlElement->setY(bounds.top()); + _qmlElement->setWidth(bounds.width()); + _qmlElement->setHeight(bounds.height()); + QMetaObject::invokeMethod(_qmlElement, "updatePropertiesFromScript", Qt::DirectConnection, Q_ARG(QVariant, properties)); } } diff --git a/interface/src/ui/overlays/QmlOverlay.h b/interface/src/ui/overlays/QmlOverlay.h index ced2b6fa1f..7f2cf5a918 100644 --- a/interface/src/ui/overlays/QmlOverlay.h +++ b/interface/src/ui/overlays/QmlOverlay.h @@ -32,10 +32,11 @@ public: void render(RenderArgs* args) override; private: + Q_INVOKABLE void qmlElementDestroyed(); Q_INVOKABLE void buildQmlElement(const QUrl& url); protected: - std::shared_ptr _qmlElement; + QQuickItem* _qmlElement{ nullptr }; }; #endif // hifi_QmlOverlay_h From aacfe927518d941ac8e024602f1266471e0ac6d3 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 28 Mar 2018 09:09:56 -0700 Subject: [PATCH 3/4] Don't trigger crashes from timers while waiting for some operation on exit --- interface/src/Application.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 968af3e298..56068fba68 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -7488,7 +7488,7 @@ DisplayPluginPointer Application::getActiveDisplayPlugin() const { return _displayPlugin; } - if (!_displayPlugin) { + if (!_aboutToQuit && !_displayPlugin) { const_cast(this)->updateDisplayMode(); Q_ASSERT(_displayPlugin); } From 9be162de95293d911cbaddfb1e772c53e4599ddf Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 28 Mar 2018 09:13:46 -0700 Subject: [PATCH 4/4] Synchronously shut down QML rendering threads --- libraries/qml/src/qml/impl/SharedObject.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/libraries/qml/src/qml/impl/SharedObject.cpp b/libraries/qml/src/qml/impl/SharedObject.cpp index f2af5bd036..d66f0f1dab 100644 --- a/libraries/qml/src/qml/impl/SharedObject.cpp +++ b/libraries/qml/src/qml/impl/SharedObject.cpp @@ -69,6 +69,7 @@ SharedObject::SharedObject() { _quickWindow->setColor(QColor(255, 255, 255, 0)); _quickWindow->setClearBeforeRendering(true); + QObject::connect(qApp, &QCoreApplication::aboutToQuit, this, &SharedObject::onAboutToQuit); } @@ -124,6 +125,7 @@ void SharedObject::setRootItem(QQuickItem* rootItem) { _renderThread->setObjectName(objectName()); _renderThread->start(); + // Create event handler for the render thread _renderObject = new RenderEventHandler(this, _renderThread); QCoreApplication::postEvent(this, new OffscreenEvent(OffscreenEvent::Initialize)); @@ -152,9 +154,16 @@ void SharedObject::destroy() { QObject::disconnect(_renderControl); QObject::disconnect(qApp); - QMutexLocker lock(&_mutex); - _quit = true; - QCoreApplication::postEvent(_renderObject, new OffscreenEvent(OffscreenEvent::Quit)); + { + QMutexLocker lock(&_mutex); + _quit = true; + QCoreApplication::postEvent(_renderObject, new OffscreenEvent(OffscreenEvent::Quit), Qt::HighEventPriority); + } + // Block until the rendering thread has stopped + // FIXME this is undesirable because this is blocking the main thread, + // but I haven't found a reliable way to do this only at application + // shutdown + _renderThread->wait(); }