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

Segfault editting TreeItem immediately after re-adding to Tree. #88889

Closed
OffsetMOSFET opened this issue Feb 27, 2024 · 5 comments · Fixed by #88917
Closed

Segfault editting TreeItem immediately after re-adding to Tree. #88889

OffsetMOSFET opened this issue Feb 27, 2024 · 5 comments · Fixed by #88917

Comments

@OffsetMOSFET
Copy link

Tested versions

Godot 4.2.2

System information

Ubuntu 22.04.4 LTS 64-bit

Issue description

Clicking to edit a TreeNode after moving it causes a segfault (signal 11). Selecting another TreeNode will prevent this from happening.

Steps to reproduce

Remove the item from the tree; add the item to the tree; click on the item to edit it.

Minimal reproduction project (MRP)

paste onto a Tree node:

extends Tree

var not_crash_item:TreeItem
var crash_item:TreeItem


func _init():
	hide_root = true
	set_anchors_preset(Control.PRESET_FULL_RECT)


func _ready():
	var root := create_item()
	
	not_crash_item = create_item(root)
	not_crash_item.set_editable(0, true)
	
	crash_item = create_item(root)
	crash_item.set_editable(0, true)

	crash_item.select(0)
	get_root().remove_child(crash_item)
	get_root().add_child(crash_item)
	not_crash_item.set_text(0, "click to prevent crash")
	crash_item.set_text(0, "click to crash")

@OffsetMOSFET
Copy link
Author

Calling .deselect before the node is removed also avoids this crash.

@akien-mga
Copy link
Member

Confirmed in latest master (a586e86). Backtrace:

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.3.dev.custom_build (a586e860e5fc382dec4ad9a0bec72f7c6684f020)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib64/libc.so.6(+0x3e9a0) [0x7f010b7b89a0] (??:0)
[2] Tree::gui_input(Ref<InputEvent> const&) (/home/akien/Godot/godot/./scene/gui/tree.cpp:3736 (discriminator 1))
[3] Control::_call_gui_input(Ref<InputEvent> const&) (/home/akien/Godot/godot/./scene/gui/control.cpp:1800)
[4] Viewport::_gui_call_input(Control*, Ref<InputEvent> const&) (/home/akien/Godot/godot/./scene/main/viewport.cpp:1612)
[5] Viewport::_gui_input_event(Ref<InputEvent>) (/home/akien/Godot/godot/./scene/main/viewport.cpp:1878 (discriminator 2))
[6] Viewport::push_input(Ref<InputEvent> const&, bool) (/home/akien/Godot/godot/./scene/main/viewport.cpp:3377 (discriminator 2))
[7] Window::_window_input(Ref<InputEvent> const&) (/home/akien/Godot/godot/./scene/main/window.cpp:1609)
[8] void call_with_variant_args_helper<Window, Ref<InputEvent> const&, 0ul>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, Callable::CallError&, IndexSequence<0ul>) (/home/akien/Godot/godot/./core/variant/binder_common.h:304 (discriminator 2))
[9] void call_with_variant_args<Window, Ref<InputEvent> const&>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, int, Callable::CallError&) (/home/akien/Godot/godot/./core/variant/binder_common.h:419)
[10] CallableCustomMethodPointer<Window, Ref<InputEvent> const&>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Godot/godot/./core/object/callable_method_pointer.h:99)
[11] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Godot/godot/./core/variant/callable.cpp:56)
[12] Variant Callable::call<Ref<InputEvent> >(Ref<InputEvent>) const (/home/akien/Godot/godot/./core/variant/variant.h:864)
[13] DisplayServerWayland::_dispatch_input_event(Ref<InputEvent> const&) (/home/akien/Godot/godot/platform/linuxbsd/wayland/display_server_wayland.cpp:93 (discriminator 2))
[14] DisplayServerWayland::dispatch_input_events(Ref<InputEvent> const&) (/home/akien/Godot/godot/platform/linuxbsd/wayland/display_server_wayland.cpp:88)
[15] Input::_parse_input_event_impl(Ref<InputEvent> const&, bool) (/home/akien/Godot/godot/./core/input/input.cpp:771)
[16] Input::flush_buffered_events() (/home/akien/Godot/godot/./core/input/input.cpp:1042)
[17] DisplayServerWayland::process_events() (/home/akien/Godot/godot/platform/linuxbsd/wayland/display_server_wayland.cpp:1176)
[18] OS_LinuxBSD::run() (/home/akien/Godot/godot/platform/linuxbsd/os_linuxbsd.cpp:936)
[19] /home/akien/Godot/godot/bin/godot.linuxbsd.editor.dev.x86_64(main+0x15a) [0x2abfb10] (/home/akien/Godot/godot/platform/linuxbsd/godot_linuxbsd.cpp:88)
[20] /lib64/libc.so.6(+0x2814a) [0x7f010b7a214a] (??:0)
[21] /lib64/libc.so.6(__libc_start_main+0x8b) [0x7f010b7a220b] (??:0)
[22] /home/akien/Godot/godot/bin/godot.linuxbsd.editor.dev.x86_64(_start+0x25) [0x2abf8f5] (??:?)
-- END OF BACKTRACE --
================================================================

@akien-mga
Copy link
Member

CC @KoBeWi @Sauermann

@mihe
Copy link
Contributor

mihe commented Feb 27, 2024

As far as I can tell the issue seems to be that the selected property of the cells in selected_item isn't set to false when selected_item is cleared as part of removing it from the tree. This then leads to selected_item not being set again (and thus left at nullptr) when you click the item after having re-added it.

Adding this code above this line fixes the issue:

for (int i = 0; i < tree->selected_item->cells.size(); i++) {
    tree->selected_item->cells.write[i].selected = false;
}

There might be a cleaner solution though.

@mihe
Copy link
Contributor

mihe commented Feb 27, 2024

I made it into a PR, in case it happens to be the preferred solution: #88917.

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.

4 participants