diff options
author | Miikka Heikkinen <miikka.heikkinen@qt.io> | 2020-12-30 14:50:49 +0200 |
---|---|---|
committer | Miikka Heikkinen <miikka.heikkinen@qt.io> | 2021-01-05 09:28:45 +0000 |
commit | bab69fff777b56c4fe9385d915eb717e379a0e8d (patch) | |
tree | 6173b455cf01ec4c1622e981eb99b73d619e72e2 /share | |
parent | 55ca28f1c7193d87e0b3c94f5887bd1e65ccac6a (diff) |
QmlPuppet: Fix crash at puppet reset
Puppet cleanup was not handled properly, so derefFromEffectItem() was
not called for all objects that needed it, causing puppet to crash
at shutdown.
Fixes: QDS-3461
Change-Id: I22c0552fe1223789fa42b276ab377d4a9e929955
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>
Diffstat (limited to 'share')
8 files changed, 45 insertions, 27 deletions
diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/nodeinstanceserver.cpp b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/nodeinstanceserver.cpp index 64d790c6a7..a4175fe992 100644 --- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/nodeinstanceserver.cpp +++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/nodeinstanceserver.cpp @@ -771,25 +771,18 @@ QList<QObject*> NodeInstanceServer::allSubObjectsForObject(QObject *object) void NodeInstanceServer::removeAllInstanceRelationships() { - // prevent destroyed() signals calling back - - foreach (ServerNodeInstance instance, m_objectInstanceHash) { + for (ServerNodeInstance &instance : m_objectInstanceHash) { if (instance.isValid()) - instance.setId(QString()); + instance.setId({}); } - //first the root object - if (rootNodeInstance().internalObject()) - rootNodeInstance().internalObject()->disconnect(); - + // First the root object + // This also cleans up all objects that have root object as ancestor rootNodeInstance().makeInvalid(); - - foreach (ServerNodeInstance instance, m_objectInstanceHash) { - if (instance.internalObject()) - instance.internalObject()->disconnect(); + // Invalidate any remaining objects + for (ServerNodeInstance &instance : m_objectInstanceHash) instance.makeInvalid(); - } m_idInstances.clear(); m_objectInstanceHash.clear(); @@ -1388,14 +1381,14 @@ void NodeInstanceServer::sendDebugOutput(DebugOutputCommand::Type type, const QS nodeInstanceClient()->debugOutput(command); } -void NodeInstanceServer::removeInstanceRelationsipForDeletedObject(QObject *object) +void NodeInstanceServer::removeInstanceRelationsipForDeletedObject(QObject *object, qint32 instanceId) { if (m_objectInstanceHash.contains(object)) { ServerNodeInstance instance = instanceForObject(object); m_objectInstanceHash.remove(object); - if (instance.instanceId() >= 0 && m_idInstances.size() > instance.instanceId()) - m_idInstances[instance.instanceId()] = ServerNodeInstance{}; + if (instanceId >= 0 && m_idInstances.size() > instanceId) + m_idInstances[instanceId] = {}; } } diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/nodeinstanceserver.h b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/nodeinstanceserver.h index f44a0047fe..fef555c06c 100644 --- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/nodeinstanceserver.h +++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/nodeinstanceserver.h @@ -204,7 +204,7 @@ public: const QString &message, const QVector<qint32> &instanceIds); - void removeInstanceRelationsipForDeletedObject(QObject *object); + void removeInstanceRelationsipForDeletedObject(QObject *object, qint32 instanceId); void incrementNeedsExtraRender(); void decrementNeedsExtraRender(); diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.cpp b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.cpp index d7fba0a8e7..0f56b809f4 100644 --- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.cpp +++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.cpp @@ -65,12 +65,7 @@ ObjectNodeInstance::ObjectNodeInstance(QObject *object) { if (object) QObject::connect(m_object.data(), &QObject::destroyed, [=] { - - /*This lambda is save because m_nodeInstanceServer - is a smartpointer and object is a dangling pointer anyway.*/ - - if (m_nodeInstanceServer) - m_nodeInstanceServer->removeInstanceRelationsipForDeletedObject(object); + handleObjectDeletion(object); }); } @@ -100,6 +95,14 @@ void ObjectNodeInstance::destroy() m_instanceId = -1; } +void ObjectNodeInstance::handleObjectDeletion(QObject *object) +{ + // We must pass the m_instanceId here, because this instance is no longer + // valid, so the wrapper ServerNodeInstance will report -1 for id. + if (m_nodeInstanceServer) + m_nodeInstanceServer->removeInstanceRelationsipForDeletedObject(object, m_instanceId); +} + void ObjectNodeInstance::setInstanceId(qint32 id) { m_instanceId = id; diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.h b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.h index 01cb1c46fc..c66365d392 100644 --- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.h +++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/objectnodeinstance.h @@ -65,7 +65,7 @@ public: virtual ~ObjectNodeInstance(); void destroy(); - //void setModelNode(const ModelNode &node); + virtual void handleObjectDeletion(QObject *object); static Pointer create(QObject *objectToBeWrapped); static QObject *createPrimitive(const QString &typeName, int majorNumber, int minorNumber, QQmlContext *context); diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp index 6879ddacdf..466bf97db8 100644 --- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp +++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5informationnodeinstanceserver.cpp @@ -915,6 +915,9 @@ Qt5InformationNodeInstanceServer::~Qt5InformationNodeInstanceServer() m_render3DEditViewTimer.stop(); m_inputEventTimer.stop(); + if (m_editView3DData.rootItem) + m_editView3DData.rootItem->disconnect(this); + for (auto view : qAsConst(m_view3Ds)) view->disconnect(); for (auto node : qAsConst(m_3DSceneMap)) @@ -922,6 +925,15 @@ Qt5InformationNodeInstanceServer::~Qt5InformationNodeInstanceServer() if (m_editView3DData.rootItem) QMetaObject::invokeMethod(m_editView3DData.rootItem, "aboutToShutDown", Qt::DirectConnection); + + if (!Internal::QuickItemNodeInstance::unifiedRenderPath()) { + if (m_editView3DData.contentItem) + designerSupport()->derefFromEffectItem(m_editView3DData.contentItem); + if (m_modelNode3DImageViewData.contentItem) + designerSupport()->derefFromEffectItem(m_modelNode3DImageViewData.contentItem); + if (m_modelNode2DImageViewData.contentItem) + designerSupport()->derefFromEffectItem(m_modelNode2DImageViewData.contentItem); + } } void Qt5InformationNodeInstanceServer::sendTokenBack() diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5nodeinstanceserver.cpp b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5nodeinstanceserver.cpp index ee1bacbc2e..a9dc087413 100644 --- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5nodeinstanceserver.cpp +++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/qt5nodeinstanceserver.cpp @@ -37,6 +37,7 @@ #include <addimportcontainer.h> #include <createscenecommand.h> #include <reparentinstancescommand.h> +#include <clearscenecommand.h> #include <QDebug> #include <QOpenGLContext> @@ -59,7 +60,8 @@ Qt5NodeInstanceServer::Qt5NodeInstanceServer(NodeInstanceClientInterface *nodeIn Qt5NodeInstanceServer::~Qt5NodeInstanceServer() { - delete quickWindow(); + NodeInstanceServer::clearScene({}); + delete m_viewData.window.data(); } QQuickView *Qt5NodeInstanceServer::quickView() const diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/quickitemnodeinstance.cpp b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/quickitemnodeinstance.cpp index 81607cbdd6..ee74ab856c 100644 --- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/quickitemnodeinstance.cpp +++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/quickitemnodeinstance.cpp @@ -59,8 +59,15 @@ QuickItemNodeInstance::QuickItemNodeInstance(QQuickItem *item) QuickItemNodeInstance::~QuickItemNodeInstance() { - if (quickItem() && checkIfRefFromEffect(instanceId())) - designerSupport()->derefFromEffectItem(quickItem()); +} + +void QuickItemNodeInstance::handleObjectDeletion(QObject *object) +{ + auto item = qobject_cast<QQuickItem *>(object); + if (item && checkIfRefFromEffect(instanceId())) + designerSupport()->derefFromEffectItem(item); + + ObjectNodeInstance::handleObjectDeletion(object); } static bool isContentItem(QQuickItem *item, NodeInstanceServer *nodeInstanceServer) diff --git a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/quickitemnodeinstance.h b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/quickitemnodeinstance.h index 2c68fe7974..b71b990986 100644 --- a/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/quickitemnodeinstance.h +++ b/share/qtcreator/qml/qmlpuppet/qml2puppet/instances/quickitemnodeinstance.h @@ -42,6 +42,7 @@ public: using WeakPointer = QWeakPointer<QuickItemNodeInstance>; ~QuickItemNodeInstance() override; + void handleObjectDeletion(QObject *object) override; static Pointer create(QObject *objectToBeWrapped); static void createEffectItem(bool createEffectItem); |