crash on removing node children and re-creating them

user4592357

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:

expand the node
click on child node
collapse the node
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?

SGaist

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.

user4592357

@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().

SGaist

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.

user4592357

@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?

SGaist

How are you processing that selection ?

user4592357

@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

SGaist

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

user4592357

@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);
}

SGaist

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

user4592357

@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.