From 9a9242817d994202a4f056a74aa3ec213f32d50a Mon Sep 17 00:00:00 2001 From: tetektoza Date: Tue, 10 Jun 2025 09:38:31 +0200 Subject: [PATCH 1/2] Core: Fix crashes during item deletion in specific selection order This patch fixes crashes that we've noticed during migration from Qt5 to Qt6 in recent months. If you select items in a tree in a specific direction or range (all, or from bottom to top) and delete them, there is a high change user will experience a crash in `testStatus` function. This problem arises because we're getting into use-after-free situation. Looking at the callstack there are a lot of calls to `itemSelectionChanged` during deletion, which takes over item creation after deletion in `TreeWidget::_slotDeleteObject`. This in turn causes `DocumentObjectItem::testStatus` to be called prematurely when we have dangling pointers in object map still. `itemSelectionChanged` signal is being transmitted because the selection range is changing as we're constantly deleting and readding certain items. Previously there was `blockSelection` call during deletion, but it turns out the signals can still be emitted even AFTER we delete the item. This had to somehow change between Qt5 and Qt6. So, to be safe, move the signal block for selection before the obj deletion loop to be sure we won't retransmit this signal during an uncertain state. --- src/Gui/Tree.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Gui/Tree.cpp b/src/Gui/Tree.cpp index b21c60c03a..d7f82de644 100644 --- a/src/Gui/Tree.cpp +++ b/src/Gui/Tree.cpp @@ -4016,6 +4016,9 @@ void TreeWidget::_slotDeleteObject(const Gui::ViewProviderDocumentObject& view, TREE_LOG("delete object " << obj->getFullName()); + // Block all selection signals during deletion to prevent cascading selection change events + // during item creation or deletion + bool lock = blockSelection(true); bool needUpdate = false; for (const auto& data : itEntry->second) { @@ -4029,13 +4032,11 @@ void TreeWidget::_slotDeleteObject(const Gui::ViewProviderDocumentObject& view, if (obj->getDocument() == doc) docItem->_ParentMap.erase(obj); - bool lock = blockSelection(true); for (auto cit = items.begin(), citNext = cit; cit != items.end(); cit = citNext) { ++citNext; (*cit)->myOwner = nullptr; delete* cit; } - blockSelection(lock); // Check for any child of the deleted object that is not in the tree, and put it // under document item. @@ -4062,6 +4063,9 @@ void TreeWidget::_slotDeleteObject(const Gui::ViewProviderDocumentObject& view, } ObjectTable.erase(itEntry); + // Restore signal state + blockSelection(lock); + if (needUpdate) _updateStatus(); } From 4c1e7835bd419b970b49ffdb75586d4e8b3c7881 Mon Sep 17 00:00:00 2001 From: tetektoza Date: Tue, 17 Jun 2025 00:15:49 +0200 Subject: [PATCH 2/2] Core: Add guard in Tree to ensure we don't process items during deletion --- src/Gui/Tree.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Gui/Tree.cpp b/src/Gui/Tree.cpp index d7f82de644..314bf1a934 100644 --- a/src/Gui/Tree.cpp +++ b/src/Gui/Tree.cpp @@ -5360,6 +5360,11 @@ enum Status { void DocumentObjectItem::testStatus(bool resetStatus, QIcon& icon1, QIcon& icon2) { + // guard against calling this during destruction when tree widget may be nullptr + if (!treeWidget()) { + return; + } + App::DocumentObject* pObject = object()->getObject(); int visible = -1;