Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

calling TreeItem set_text() on a TreeItem that was removed via remove_child() causes debugging to end without any error message #85650

Closed
UltyX opened this issue Dec 2, 2023 · 5 comments · Fixed by #86028

Comments

@UltyX
Copy link

UltyX commented Dec 2, 2023

Godot version

4.2 Stable

System information

Linux

Issue description

Calling TreeItem set_text() on a TreeItem that was removed via remove_child() causes debugging to end and the application to close without any error message.

Steps to reproduce

Run the included example to get the debugger to crash. Comment out "treeItem.remove_child(subTreeItem)" for normal behaviour.

Minimal reproduction project

treeItemCrash.zip

@kleonc
Copy link
Member

kleonc commented Dec 2, 2023

It crashes here:

_changed_notify(p_column);

godot/scene/gui/tree.cpp

Lines 72 to 74 in d76c1d0

void TreeItem::_changed_notify(int p_cell) {
tree->item_changed(p_cell, this);
}

@AThousandShips
Copy link
Member

A check in either place should be sufficient, possible leftover from allowing handling items without a tree and the previous assumption tree items were always in a tree

@rsubtil
Copy link
Contributor

rsubtil commented Dec 2, 2023

Yup, crashing specifically here at columns.write[p_column]..., seems like it only needs an extra check to ensure p_column < columns.size():

godot/scene/gui/tree.cpp

Lines 4347 to 4353 in d76c1d0

void Tree::item_changed(int p_column, TreeItem *p_item) {
if (p_item != nullptr && p_column >= 0 && p_column < p_item->cells.size()) {
p_item->cells.write[p_column].dirty = true;
columns.write[p_column].cached_minimum_width_dirty = true;
}
queue_redraw();
}

@AThousandShips
Copy link
Member

That's not the error position, the tree is null, so it crashes when it calls the method on tree

@rsubtil
Copy link
Contributor

rsubtil commented Dec 2, 2023

That's not the error position, the tree is null, so it crashes when it calls the method on tree

Yeah my bad, I failed to notice tree was null before going into this call 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants