From efd33c846505f29de2376d2128a0eb5f0731df80 Mon Sep 17 00:00:00 2001 From: jrheinlaender Date: Wed, 17 Apr 2013 19:49:55 +0430 Subject: [PATCH] Added check in Tree.cpp DocumentItem::slotChangeObject to check for invalid pointers returned by claimChildren() --- src/App/Document.cpp | 10 ++++++++++ src/App/Document.h | 2 ++ src/Gui/Tree.cpp | 44 +++++++++++++++++++++++++------------------- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index f1cf43b00..8238964c4 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -2294,6 +2294,16 @@ DocumentObject * Document::getObject(const char *Name) const return 0; } +// Note: This method is only used in Tree.cpp slotChangeObject(), see explanation there +const bool Document::isIn(const DocumentObject *pFeat) const +{ + for (std::map::const_iterator o = d->objectMap.begin(); o != d->objectMap.end(); o++) + if (o->second == pFeat) + return true; + + return false; +} + const char * Document::getObjectName(DocumentObject *pFeat) const { std::map::const_iterator pos; diff --git a/src/App/Document.h b/src/App/Document.h index 1402f7cc4..7d1b1d921 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -192,6 +192,8 @@ public: DocumentObject *getActiveObject(void) const; /// Returns a Object of this document DocumentObject *getObject(const char *Name) const; + /// Returns true if the DocumentObject is contained in this document + const bool isIn(const DocumentObject *pFeat) const; /// Returns a Name of an Object or 0 const char *getObjectName(DocumentObject *pFeat) const; /// Returns a Name of an Object or 0 diff --git a/src/Gui/Tree.cpp b/src/Gui/Tree.cpp index b267fd12b..44fb60266 100644 --- a/src/Gui/Tree.cpp +++ b/src/Gui/Tree.cpp @@ -943,25 +943,31 @@ void DocumentItem::slotChangeObject(const Gui::ViewProviderDocumentObject& view) std::string objectName = obj->getNameInDocument(); std::map::iterator it = ObjectMap.find(objectName); if (it != ObjectMap.end()) { - // use new grouping style - std::set children; - std::vector group = view.claimChildren(); - for (std::vector::iterator jt = group.begin(); jt != group.end(); ++jt) { - if (*jt) { - const char* internalName = (*jt)->getNameInDocument(); - if (internalName) { - std::map::iterator kt = ObjectMap.find(internalName); - if (kt != ObjectMap.end()) { - children.insert(kt->second); - QTreeWidgetItem* parent = kt->second->parent(); - if (parent && parent != it->second) { - if (it->second != kt->second) { - int index = parent->indexOfChild(kt->second); - parent->takeChild(index); - it->second->addChild(kt->second); - } - else { - Base::Console().Warning("Gui::DocumentItem::slotChangedObject(): Object references to itself.\n"); + // use new grouping style + std::set children; + std::vector group = view.claimChildren(); + for (std::vector::iterator jt = group.begin(); jt != group.end(); ++jt) { + if ((*jt) && view.getObject()->getDocument()->isIn(*jt)){ + // Note: It is possible that we receive an invalid pointer from claimChildren(), e.g. if multiple properties + // were changed in a transaction and slotChangedObject() is triggered by one property being reset + // before the invalid pointer has been removed from another. Currently this happens for PartDesign::Body + // when cancelling a new feature in the dialog. First the new feature is deleted, then the Tip property is + // reset, but claimChildren() accesses the Model property which still contains the pointer to the deleted feature + const char* internalName = (*jt)->getNameInDocument(); + if (internalName) { + std::map::iterator kt = ObjectMap.find(internalName); + if (kt != ObjectMap.end()) { + children.insert(kt->second); + QTreeWidgetItem* parent = kt->second->parent(); + if (parent && parent != it->second) { + if (it->second != kt->second) { + int index = parent->indexOfChild(kt->second); + parent->takeChild(index); + it->second->addChild(kt->second); + } + else { + Base::Console().Warning("Gui::DocumentItem::slotChangedObject(): Object references to itself.\n"); + } } } }