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.

Sign up for GitHub

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

Jump to bottom

[4.4 dev 7] Crash to Desktop on choosing skeleton node #100666

Closed
Sove67 opened this issue Dec 20, 2024 · 3 comments · Fixed by #100670
Closed

[4.4 dev 7] Crash to Desktop on choosing skeleton node #100666

Sove67 opened this issue Dec 20, 2024 · 3 comments · Fixed by #100670
Assignees
@hpvb
Labels
bug crash topic:animation topic:editor
Milestone
4.4

Comments

@Sove67
Copy link

Sove67 commented Dec 20, 2024

Tested versions

  • reproducible in 4.4 Dev 7, mono version

System information

Godot v4.4.dev7.mono - Windows 10 (build 19045) - Multi-window, 1 monitor - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1070 (NVIDIA; 32.0.15.6094) - Intel(R) Core(TM) i5-7640X CPU @ 4.00GHz (4 threads)

Issue description

While making a MRP for another bug, I manage to crash the editor when assigning the skeleton of a mesh.

Steps to reproduce

Edit the attached MRP. Click on the assigned Node3D in the Skeleton variable to pick a new node.
Image

A selection window will appear, frozen and blank white. In a moment, Godot will crash.

Minimal reproduction project (MRP)

CTD-example.zip
@akien-mga akien-mga added this to the 4.4 milestone Dec 20, 2024
@akien-mga
Copy link
Member

It's a regression in dev7, probably from #99700 judging by the stack (overflow?) trace. CC @hpvb

(gdb) bt
#0  0x000000000bd5bf72 in NodePath::NodePath (this=<error reading variable: Cannot access memory at address 0x7fffff7fefc8>, p_path=<error reading variable: Cannot access memory at address 0x7fffff7fefc0>)
    at ./core/string/node_path.cpp:393
#1  0x000000000b9b6f02 in Variant::reference (this=0x7fffff7ff290, p_variant=...) at ./core/variant/variant.cpp:1242
#2  0x000000000b9bd813 in Variant::Variant (this=0x7fffff7ff290, p_variant=...) at ./core/variant/variant.cpp:2919
#3  0x0000000009b9bed0 in TreeItem::get_metadata (this=0x2c6df210, p_column=0) at ./scene/gui/tree.cpp:656
#4  0x0000000008efd792 in SceneTreeEditor::_selected_changed (this=0x2c638d40) at ./editor/gui/scene_tree_editor.cpp:1162
#5  0x0000000008d7539f in call_with_variant_args_helper<SceneTreeEditor>(SceneTreeEditor*, void (SceneTreeEditor::*)(), Variant const**, Callable::CallError&, IndexSequence<>) (p_instance=0x2c638d40, 
    p_method=(void (SceneTreeEditor::*)(SceneTreeEditor * const)) 0x8efd71c <SceneTreeEditor::_selected_changed()>, p_args=0x0, r_error=...) at ./core/variant/binder_common.h:304
#6  0x0000000008d7024c in call_with_variant_args<SceneTreeEditor> (p_instance=0x2c638d40, p_method=(void (SceneTreeEditor::*)(SceneTreeEditor * const)) 0x8efd71c <SceneTreeEditor::_selected_changed()>, 
    p_args=0x0, p_argcount=0, r_error=...) at ./core/variant/binder_common.h:418
#7  0x0000000008d589f8 in CallableCustomMethodPointer<SceneTreeEditor, void>::call (this=0x1fea6420, p_arguments=0x0, p_argcount=0, r_return_value=..., r_call_error=...)
    at ./core/object/callable_method_pointer.h:105
#8  0x000000000b9ac197 in Callable::callp (this=0x7fffff7ff520, p_arguments=0x0, p_argcount=0, r_return_value=..., r_call_error=...) at ./core/variant/callable.cpp:57
#9  0x000000000bcfcce7 in Object::emit_signalp (this=0x2c639a20, p_name=..., p_args=0x0, p_argcount=0) at ./core/object/object.cpp:1199
#10 0x000000000985653e in Node::emit_signalp (this=0x2c639a20, p_name=..., p_args=0x0, p_argcount=0) at ./scene/main/node.cpp:4021
#11 0x0000000006e69b99 in Object::emit_signal<>(StringName const&) (this=0x2c639a20, p_name=...) at ./core/object/object.h:922
#12 0x0000000009bacd35 in Tree::select_single_item (this=0x2c639a20, p_selected=0x2c6df210, p_current=0x2c6df210, p_col=0, p_prev=0x0, r_in_range=0x0, p_force_deselect=false) at ./scene/gui/tree.cpp:2807
#13 0x0000000009bb8149 in Tree::item_selected (this=0x2c639a20, p_column=0, p_item=0x2c6df210) at ./scene/gui/tree.cpp:4719
#14 0x0000000009b97e55 in TreeItem::_cell_selected (this=0x2c6df210, p_cell=0) at ./scene/gui/tree.cpp:91
#15 0x0000000009b9e1ba in TreeItem::select (this=0x2c6df210, p_column=0) at ./scene/gui/tree.cpp:1253
#16 0x0000000008efa4a9 in SceneTreeEditor::_update_node (this=0x2c638d40, p_node=0x2bfccd80, p_item=0x2c6df210, p_part_of_subscene=false) at ./editor/gui/scene_tree_editor.cpp:621
#17 0x0000000008ef69a1 in SceneTreeEditor::_update_node_subtree (this=0x2c638d40, p_node=0x2bfccd80, p_parent=0x0, p_force=true) at ./editor/gui/scene_tree_editor.cpp:334
#18 0x0000000008efc2ee in SceneTreeEditor::_update_tree (this=0x2c638d40, p_scroll_to_selected=false) at ./editor/gui/scene_tree_editor.cpp:928
#19 0x0000000008efec88 in SceneTreeEditor::set_selected (this=0x2c638d40, p_node=0x0, p_emit_selected=false) at ./editor/gui/scene_tree_editor.cpp:1319
#20 0x0000000008ef6d89 in SceneTreeEditor::_update_node_subtree (this=0x2c638d40, p_node=0x2bfccd80, p_parent=0x0, p_force=true) at ./editor/gui/scene_tree_editor.cpp:372
#21 0x0000000008efc2ee in SceneTreeEditor::_update_tree (this=0x2c638d40, p_scroll_to_selected=false) at ./editor/gui/scene_tree_editor.cpp:928
#22 0x0000000008efec88 in SceneTreeEditor::set_selected (this=0x2c638d40, p_node=0x0, p_emit_selected=false) at ./editor/gui/scene_tree_editor.cpp:1319
#23 0x0000000008ef6d89 in SceneTreeEditor::_update_node_subtree (this=0x2c638d40, p_node=0x2bfccd80, p_parent=0x0, p_force=true) at ./editor/gui/scene_tree_editor.cpp:372
#24 0x0000000008efc2ee in SceneTreeEditor::_update_tree (this=0x2c638d40, p_scroll_to_selected=false) at ./editor/gui/scene_tree_editor.cpp:928
#25 0x0000000008efec88 in SceneTreeEditor::set_selected (this=0x2c638d40, p_node=0x0, p_emit_selected=false) at ./editor/gui/scene_tree_editor.cpp:1319

...

#54158 0x0000000008ef6d89 in SceneTreeEditor::_update_node_subtree (this=0x2c638d40, p_node=0x2bfccd80, p_parent=0x0, p_force=true) at ./editor/gui/scene_tree_editor.cpp:372
#54159 0x0000000008efc2ee in SceneTreeEditor::_update_tree (this=0x2c638d40, p_scroll_to_selected=false) at ./editor/gui/scene_tree_editor.cpp:928
#54160 0x0000000008efec88 in SceneTreeEditor::set_selected (this=0x2c638d40, p_node=0x0, p_emit_selected=false) at ./editor/gui/scene_tree_editor.cpp:1319
#54161 0x0000000008ef6d89 in SceneTreeEditor::_update_node_subtree (this=0x2c638d40, p_node=0x2bfccd80, p_parent=0x0, p_force=true) at ./editor/gui/scene_tree_editor.cpp:372
#54162 0x0000000008efc2ee in SceneTreeEditor::_update_tree (this=0x2c638d40, p_scroll_to_selected=false) at ./editor/gui/scene_tree_editor.cpp:928
#54163 0x0000000008d7583d in call_with_variant_args_helper<SceneTreeEditor, bool, 0ul> (p_instance=0x2c638d40, 
    p_method=(void (SceneTreeEditor::*)(SceneTreeEditor * const, bool)) 0x8efc0e6 <SceneTreeEditor::_update_tree(bool)>, p_args=0x7fffffffcbc0, r_error=...) at ./core/variant/binder_common.h:304
#54164 0x0000000008d7064d in call_with_variant_args<SceneTreeEditor, bool> (p_instance=0x2c638d40, 
    p_method=(void (SceneTreeEditor::*)(SceneTreeEditor * const, bool)) 0x8efc0e6 <SceneTreeEditor::_update_tree(bool)>, p_args=0x7fffffffcbc0, p_argcount=1, r_error=...) at ./core/variant/binder_common.h:418
#54165 0x0000000008d5a780 in CallableCustomMethodPointer<SceneTreeEditor, void, bool>::call (this=0x2b0be470, p_arguments=0x7fffffffcbc0, p_argcount=1, r_return_value=..., r_call_error=...)
    at ./core/object/callable_method_pointer.h:105
#54166 0x000000000b9ac197 in Callable::callp (this=0xe6e6f98, p_arguments=0x7fffffffcbc0, p_argcount=1, r_return_value=..., r_call_error=...) at ./core/variant/callable.cpp:57
#54167 0x000000000bcf5673 in CallQueue::_call_function (this=0xc93c320, p_callable=..., p_args=0xe6e6fb0, p_argcount=1, p_show_error=true) at ./core/object/message_queue.cpp:220
#54168 0x000000000bcf5a08 in CallQueue::flush (this=0xc93c320) at ./core/object/message_queue.cpp:268
#54169 0x0000000009859f1c in SceneTree::physics_process (this=0xf336440, p_time=0.016666666666666666) at ./scene/main/scene_tree.cpp:546
#54170 0x00000000066cfdf7 in Main::iteration () at main/main.cpp:4398
#54171 0x000000000660c500 in OS_LinuxBSD::run (this=0x7fffffffcf20) at platform/linuxbsd/os_linuxbsd.cpp:962
#54172 0x0000000006604c71 in main (argc=2, argv=0x7fffffffd588) at platform/linuxbsd/godot_linuxbsd.cpp:85

@akien-mga akien-mga moved this from Unassessed to Release Blocker in 4.x Release Blockers Dec 20, 2024
@akien-mga akien-mga moved this from Release Blocker to Immediate Blocker in 4.x Release Blockers Dec 20, 2024
@hpvb
Copy link
Member

hpvb commented Dec 20, 2024

Will have a look

@hpvb hpvb mentioned this issue Dec 20, 2024
Merged
@hpvb
Copy link
Member

hpvb commented Dec 20, 2024

Fixed in #100670

@AThousandShips AThousandShips marked this as a duplicate of #100840 Dec 27, 2024
tGautot pushed a commit to tGautot/godot that referenced this issue Feb 5, 2025
@hpvb @tGautot
Don't recursively call _update_tree
1ac5522 
When a node was previously selected and the test "selected == p_node"
was true the code would use set_selected() to change the selection to
nullptr. However, if the tree is dirty, which is always true in this
codepath, this would lead to a recursive call to _update_tree()
ultimately leading to a crash due to us running out of stack.

This fixes godotengine#100666
tGautot pushed a commit to tGautot/godot that referenced this issue Feb 5, 2025
@hpvb @tGautot
Don't recursively call _update_tree
1d01e16 
When a node was previously selected and the test "selected == p_node"
was true the code would use set_selected() to change the selection to
nullptr. However, if the tree is dirty, which is always true in this
codepath, this would lead to a recursive call to _update_tree()
ultimately leading to a crash due to us running out of stack.

This fixes godotengine#100666
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hpvb hpvb

Labels
bug crash topic:animation topic:editor
Projects
4.x Release Blockers
Status: Immediate Blocker
Milestone
4.4
Development

Successfully merging a pull request may close this issue.

Don't recursively call _update_tree hpvb/godot
4 participants
@hpvb @akien-mga @Sove67 @AThousandShips