Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

crash on removing node children and re-creating them



  • i have a two-level tree view, where top level items have child items:
    a
    |-b
    |-c

    the second level nodes should be created upon top level node expansion. in order not to append all nodes to existing ones upon every expansion, i first clear the node and then re-populate its children:

    void Tree::onNodeExpanded(const QModelIndex &index)
    {
    	static_cast<TreeNode *>(index.internalPointer())->onExpanded();
    	static_cast<TreeModel *>(getTreeModel())->updateModel();
    }
    
    void TreeNode::onExpanded()
    {
    	clear();
    	populate();
    }
    

    where

    void TreeNode::clear()
    {
    	for (auto pNode : m_children)
    	{
    		delete pNode;
    		pNode = nullptr;
    	}
    	m_children.clear();
    }
    void TreeNode::populate()
    {
    	// create the nodes
    }
    

    with this implementation, the app crashes with this scenario:

    1. expand the node
    2. click on child node
    3. collapse the node
    4. expand the node <-- crashes here

    the stack trace is weird:

    TreeModel::nodeRow(TreeNode*) const () from ...
    TreeModel::parent(QModelIndex const&) const () from ...
    QPersistentModelIndex::parent() const () from /lib/linux64/libQt5Core.so.5
    QItemSelectionModel::isColumnSelected(int, QModelIndex const&) const () from /lib/linux64/libQt5Core.so.5
    ?? () from /lib/linux64/libQt5Widgets.so.5
    QHeaderView::paintSection(QPainter*, QRect const&, int) const () from /lib/linux64/libQt5Widgets.so.5
    QHeaderView::paintEvent(QPaintEvent*) () from /lib/linux64/libQt5Widgets.so.5
    QWidget::event(QEvent*) () from /lib/linux64/libQt5Widgets.so.5
    QFrame::event(QEvent*) () from /lib/linux64/libQt5Widgets.so.5
    QAbstractItemView::viewportEvent(QEvent*) () from /lib/linux64/libQt5Widgets.so.5
    

    when i replace the clear() call with the below code, there is no crash:

    int childNodes = childCount();
    for (auto idx = 0; idx < childNodes; ++idx)
    	removeChild(getChild(0));
    

    where

    void TreeNode::removeChild(TreeNode *pNode)
    {
    	m_children.erase(
    		std::remove(m_children.begin(), m_children.end(), pNode), m_children.end());
    }
    TreeNode *TreeNode::getChild(int index)
    {
    	return (index >= 0 && index < childCount()) ? m_children[index] : nullptr;
    }
    

    any idea what's wrong here?


  • Lifetime Qt Champion

    Hi,

    From the looks of if, you are brute deleting your model underlying data and you don't make any validity check before calling methods on these object that you cast.



  • @sgaist
    yes i delete the nodes on purpose. what's wrong with that?

    you don't make any validity check before calling methods on these object that you cast.

    i don't delete the node itself, why check it? it's valid. the updateModel() just emits layoutChnaged().


  • Lifetime Qt Champion

    As you have discovered, you can't just nuke your data. There's a logic to follow to:

    • Make the model aware that something is going to be removed so the view gets notified to not access these data.
    • Remove the data cleanely
    • Make the model aware that you are done so the view can know it has to refresh.


  • @sgaist
    okay i get it. now i use this:

    void TreeModel::removeChildren(const QModelIndex &index)
    {
    	auto node = index.isValid() ? static_cast<TreeNode *>(index.internalPointer()) : getRootNode();
    	if (node->hasChildren())
    	{
    		beginRemoveRows(index, 0, node->childCount() - 1);
    		node->clear();
    		endRemoveRows();
    	}
    
    	updateModel();
    }
    
    void TreeNode::onExpanded()
    {
    	auto pTreeModel = ...->getTreeView()->getTree()->getTreeModel();
    
    	auto index = pTreeModel->index(this);
    	pTreeModel->removeChildren(index);
    
    	populate();
    }
    

    now, when i repeat the above scenario (expand the node, click on child node, collapse the node, expand the node), everything is fine. but now, at last step when i expand the node again, that node is selected. which i don't need (cause i process selection too).
    i tried to ...->getTreeView()->getTree()->selectionModel()->clear(); at the end of onExpanded(), but the selection changed signal is still emitted. should i just block signals?


  • Lifetime Qt Champion

    How are you processing that selection ?



  • @sgaist
    the slot does something, and emits a signal itself which is caught by another object
    more importantly: why is selection updated when i delete a selected node (i.e. the child node)? when i don't select a child node, then upon second expansion the parent node is not selected


  • Lifetime Qt Champion

    Not knowing what your code does, I can't answer that one.



  • @sgaist
    i don't perform selection programmatically, these are my connections:

    connect(selectionModel(), &QItemSelectionModel::selectionChanged, this, &Tree::onSelectionChanged);
    connect(this, &QTreeView::expanded, this, &Tree::onNodeExpanded);
    connect(this, &QTreeView::collapsed, this, &Tree::onNodeCollapsed);
    

    and then

    void Tree::onSelectionChanged(const QItemSelection &selection, const QItemSelection &deselection)
    {
    	const auto &selectedIndexes = selection.indexes();
    	if (!selectedIndexes.isEmpty())
    	{
    		auto pNode = static_cast<TreeNode *>(selectedIndexes.at(0).internalPointer());
    		pNode->onSelected(selection.indexes(), deselection.indexes(), selectionModel()->selectedIndexes());
    		emit nodeSelected(pNode);
    	}
    }
    
    void Tree::onNodeExpanded(const QModelIndex &index)
    {
    	static_cast<TreeNode *>(index.internalPointer())->onExpanded();
    	static_cast<TreeModel *>(getTreeModel())->updateModel();
    }
    
    void Tree::onNodeCollapsed(const QModelIndex &index)
    {
    	static_cast<TreeNode *>(index.internalPointer())->onCollapsed();
    	static_cast<TreeModel *>(getTreeModel())->updateModel();
    }
    

    node's onSelected():

    void TreeNode::onSelected(const QModelIndexList &/* newSelectedIndexes */,
    	const QModelIndexList &/* deselectedIndexes */,
    	const QModelIndexList &/* allSelectedIndexes */)
    {
    	QList<QVariant> columnsData;
    	// ...
    	setColumnsData(columnsData);
    }
    

  • Lifetime Qt Champion

    You seem to be doing lots of stuff here. Can you explain roughly what happens with them ?



  • @sgaist
    in short, the tree node has columns.
    when the node is selected, i fill those columns.
    when the node is expanded, i create child nodes, and the child nodes fill their columns.


Log in to reply