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.
This commit is contained in:
@@ -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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user