Editor crashes upon adding arguments to BTCallMethod

[Rubonnek]

Currently the Editor is crashing when attempting to add arguments to BTCallMethod, either when increasing the argument array size or clicking Add Element.

Internally the crash happens due to derefencing a null Object pointer.

Here's the backtrace:

#0  Object::get (this=0x0, p_name=..., r_valid=r_valid@entry=0x0) at core/object/object.cpp:317
#1  0x00005555582bd3b0 in EditorProperty::get_edited_property_value (this=0x55557673bea0) at ./editor/editor_inspector.h:162
#2  EditorPropertyBBParam::_get_edited_param (this=this@entry=0x55557673bea0) at modules/limboai/editor/editor_property_bb_param.cpp:44
#3  0x00005555582bd646 in EditorPropertyBBParam::_notification (this=this@entry=0x55557673bea0, p_what=p_what@entry=45) at modules/limboai/editor/editor_property_bb_param.cpp:313
#4  0x00005555582c00cb in EditorPropertyBBParam::_notificationv (this=0x55557673bea0, p_notification=45, p_reversed=false) at modules/limboai/editor/editor_property_bb_param.h:32
#5  0x000055555a90ad76 in Object::notification (this=0x55557673bea0, p_notification=p_notification@entry=45, p_reversed=p_reversed@entry=false) at core/object/object.cpp:837
#6  0x0000555559e46dca in ThemeOwner::_owner_context_changed (this=this@entry=0x555576b965c0) at scene/theme/theme_owner.cpp:101
#7  0x0000555559e471fe in ThemeOwner::set_owner_context (this=0x555576b965c0, p_context=<optimized out>, p_propagate=true) at scene/theme/theme_owner.cpp:87
#8  0x0000555559526417 in Control::set_theme_context (this=0x55557673bea0, p_context=0x55556eab2aa0, p_propagate=80) at scene/gui/control.cpp:2472
#9  0x000055555952a3a6 in Control::_notification (this=0x55557673bea0, p_notification=10) at scene/gui/control.cpp:3184
#10 0x0000555557cd3881 in Control::_notificationv (this=0x55557673bea0, p_notification=10, p_reversed=false) at ./scene/gui/control.h:48
#11 0x0000555558158fb0 in Container::_notificationv (this=0x55557673bea0, p_notification=10, p_reversed=false) at ./scene/gui/container.h:37
#12 0x00005555582bf1f8 in EditorProperty::_notificationv (this=0x55557673bea0, p_notification=10, p_reversed=p_reversed@entry=false) at ./editor/editor_inspector.h:58
#13 0x00005555582c00dc in EditorPropertyBBParam::_notificationv (this=0x55557673bea0, p_notification=10, p_reversed=false) at modules/limboai/editor/editor_property_bb_param.h:32
#14 0x000055555a90ad76 in Object::notification (this=this@entry=0x55557673bea0, p_notification=p_notification@entry=10, p_reversed=p_reversed@entry=false) at core/object/object.cpp:837
#15 0x00005555594107c2 in Node::_propagate_enter_tree (this=this@entry=0x55557673bea0) at scene/main/node.cpp:262
#16 0x0000555559410afe in Node::_set_tree (this=0x55557673bea0, p_tree=0x55555f6334d0) at scene/main/node.cpp:3011
#17 0x0000555559411c0a in Node::_add_child_nocheck (this=this@entry=0x555576bdadf0, p_child=<optimized out>, p_child@entry=0x55557673bea0, p_name=..., p_internal_mode=p_internal_mode@entry=Node::INTERNAL_MODE_DISABLED) at scene/main/node.cpp:1404
#18 0x0000555559415b59 in Node::add_child (this=0x555576bdadf0, p_child=0x55557673bea0, p_force_readable_name=<optimized out>, p_internal=Node::INTERNAL_MODE_DISABLED) at scene/main/node.cpp:1436
#19 0x000055555941f31b in Node::add_sibling (this=0x555576dd8d50, p_sibling=0x55557673bea0, p_force_readable_name=80, p_force_readable_name@entry=false) at scene/main/node.cpp:1446
#20 0x0000555558a24b4f in EditorPropertyArray::update_property (this=0x55557696b430) at editor/editor_properties_array_dict.cpp:410
#21 0x0000555558a216ec in EditorPropertyArray::_length_changed (this=0x55557696b430, p_page=1) at editor/editor_properties_array_dict.cpp:609
#22 0x0000555558a2e1d9 in call_with_variant_args_helper<EditorPropertyArray, double, 0ul> (p_instance=<optimized out>, p_method=<optimized out>, p_args=<optimized out>, r_error=...) at ./core/variant/binder_common.h:304
#23 0x0000555558a2e21a in call_with_variant_args<EditorPropertyArray, double> (p_instance=<optimized out>, p_method=<optimized out>, p_args=p_args@entry=0x7fffffffae20, p_argcount=p_argcount@entry=1, r_error=...) at ./core/variant/binder_common.h:418
#24 0x0000555558a2e2b9 in CallableCustomMethodPointer<EditorPropertyArray, double>::call (this=0x555576c313c0, p_arguments=0x7fffffffae20, p_argcount=1, r_return_value=..., r_call_error=...) at ./core/object/callable_method_pointer.h:98
#25 0x000055555a721472 in Callable::callp (this=this@entry=0x555577058130, p_arguments=p_arguments@entry=0x7fffffffae20, p_argcount=p_argcount@entry=1, r_return_value=..., r_call_error=...) at core/variant/callable.cpp:56
#26 0x000055555a911431 in Object::emit_signalp (this=0x555576ef7df0, p_name=..., p_args=0x7fffffffae20, p_argcount=1) at core/object/object.cpp:1134
#27 0x000055555942389c in Node::emit_signalp (this=0x555576ef7df0, p_name=..., p_args=0x7fffffffae20, p_argcount=1) at scene/main/node.cpp:3762
#28 0x0000555558235c6e in Object::emit_signal<double> (this=0x555576ef7df0, p_name=...) at ./core/object/object.h:923
#29 0x000055555961ae0d in Range::_value_changed_notify (this=0x555576ef7df0) at scene/gui/range.cpp:48
#30 0x000055555961aea9 in Range::Shared::emit_value_changed (this=<optimized out>) at scene/gui/range.cpp:58
#31 0x000055555961af34 in Range::set_value (this=this@entry=0x555576ef7df0, p_val=<optimized out>) at scene/gui/range.cpp:92
#32 0x0000555558bfc6a9 in EditorSpinSlider::gui_input (this=0x555576ef7df0, p_event=...) at editor/gui/editor_spin_slider.cpp:65
#33 0x000055555952a118 in Control::_call_gui_input (this=this@entry=0x555576ef7df0, p_event=...) at scene/gui/control.cpp:1797
#34 0x00005555594662d4 in Viewport::_gui_call_input (this=this@entry=0x55555f6339e0, p_control=<optimized out>, p_input=...) at scene/main/viewport.cpp:1603
#35 0x0000555559473b59 in Viewport::_gui_input_event (this=0x55555f6339e0, p_event=...) at scene/main/viewport.cpp:1835
#36 0x0000555559477c99 in Viewport::push_input (this=0x55555f6339e0, p_event=..., p_local_coords=80, p_local_coords@entry=false) at scene/main/viewport.cpp:3371
#37 0x0000555559499ea7 in Window::_window_input (this=0x55555f6339e0, p_ev=...) at scene/main/window.cpp:1620
#38 0x00005555594ba032 in call_with_variant_args_helper<Window, Ref<InputEvent> const&, 0ul> (p_instance=<optimized out>, p_method=<optimized out>, p_args=<optimized out>, r_error=...) at ./core/variant/binder_common.h:304
#39 0x00005555594ba092 in call_with_variant_args<Window, Ref<InputEvent> const&> (p_instance=<optimized out>, p_method=<optimized out>, p_args=p_args@entry=0x7fffffffd3d0, p_argcount=p_argcount@entry=1, r_error=...) at ./core/variant/binder_common.h:418
#40 0x00005555594ba131 in CallableCustomMethodPointer<Window, Ref<InputEvent> const&>::call (this=0x55556eac20b0, p_arguments=0x7fffffffd3d0, p_argcount=1, r_return_value=..., r_call_error=...) at ./core/object/callable_method_pointer.h:98
#41 0x000055555a721472 in Callable::callp (this=this@entry=0x7fffffffd460, p_arguments=p_arguments@entry=0x7fffffffd3d0, p_argcount=p_argcount@entry=1, r_return_value=..., r_call_error=...) at core/variant/callable.cpp:56
#42 0x00005555578868ae in Callable::call<Ref<InputEvent> > (this=this@entry=0x7fffffffd460) at ./core/variant/variant.h:863
#43 0x000055555787d9e1 in DisplayServerX11::_dispatch_input_event (this=<optimized out>, p_event=...) at platform/linuxbsd/x11/display_server_x11.cpp:4037
#44 0x000055555787da93 in DisplayServerX11::_dispatch_input_events (p_event=...) at platform/linuxbsd/x11/display_server_x11.cpp:4013
#45 0x000055555a6e32ea in Input::_parse_input_event_impl (this=this@entry=0x55555d769320, p_event=..., p_is_emulated=p_is_emulated@entry=false) at core/input/input.cpp:770
#46 0x000055555a6e3fe5 in Input::flush_buffered_events (this=0x55555d769320) at core/input/input.cpp:1041
#47 0x0000555557880ab9 in DisplayServerX11::process_events (this=0x55555dfba4a0) at platform/linuxbsd/x11/display_server_x11.cpp:5138
#48 0x000055555785e8b3 in OS_LinuxBSD::run (this=this@entry=0x7fffffffd820) at platform/linuxbsd/os_linuxbsd.cpp:941
#49 0x000055555785d142 in main (argc=<optimized out>, argv=0x7fffffffde28) at platform/linuxbsd/godot_linuxbsd.cpp:86

[limbonaut]

I can't reproduce it in both module and extension. Which LimboAI revision, Godot version and platform do you use?

[Rubonnek]

Godot: e42141fe8a1f6ec4cf910a02defc2d6ade58cb1a
LimboAI: 5909735

[Rubonnek]

Recently I've been stumbling upon some race conditions within the Godot Editor itself -- let me take a closer look. I wonder if it's one of those.

[limbonaut]

I'm currently targeting 4.2 branch for development, and planning to switch to master after 4.3 gets to beta3/rc. So I didn't test it vs bleeding edge. There might be issues.

[Rubonnek]

That's alright. I'll try to help close the gap against the bleeding edge a bit if I can. I want to stick to the bleeding edge version because some race conditions my hardware was stumbling upon have been fixed there.

[Rubonnek]

I'm currently targeting 4.2 branch for development

I can't reproduce the crash when using that branch either. Something changed upstream. I'll bisect the issue.

Edit: I don't think it's a race condition either -- the reports from ThreadSanitizer are most likely issues with TextServerAdvanced which isn't currently locking its threads correctly. Those race conditions are not caused by LimboAI as far as I can tell.

[Rubonnek]

The offending commit is godotengine/godot#87085 -- it either made the issue apparent or introduced a new one. I think it's the latter.

[limbonaut]

So it crashes somewhere in the BBParam property editor, and it's related to theming. Maybe, it's worth checking the code that deals with theming in the property editor itself?

[Rubonnek]

I'm not sure what's really happening now -- I disabled ASLR to track some memory values and now I'm also crashing on a commit that previously worked. I need to bisect again.

What I've been able to pinpoint so far is that EditorProperty::set_object_and_property does not get called when updating the array size, which is why EditorPropertyBBParam::object is null and crashes when trying to dereference it I believe.

Edit: this is the call stack that happens on 4.2.1-stable but not on the latest commit:

#0  EditorProperty::set_object_and_property (this=this@entry=0x55557721ffc0, p_object=0x555576c3f250, p_property=...) at editor/editor_inspector.cpp:854
#1  0x000055555890edf0 in EditorPropertyArray::update_property (this=0x555576c3e4b0) at editor/editor_properties_array_dict.cpp:392
#2  0x000055555890d464 in EditorPropertyArray::_length_changed (this=0x555576c3e4b0, p_page=1) at editor/editor_properties_array_dict.cpp:594
#3  0x00005555589199d9 in call_with_variant_args_helper<EditorPropertyArray, double, 0ul> (p_instance=<optimized out>, p_method=<optimized out>, p_args=<optimized out>, r_error=...) at ./core/variant/binder_common.h:303
#4  0x0000555558919a1a in call_with_variant_args<EditorPropertyArray, double> (p_instance=<optimized out>, p_method=<optimized out>, p_args=p_args@entry=0x7fffffffae20, p_argcount=p_argcount@entry=1, r_error=...) at ./core/variant/binder_common.h:417
#5  0x0000555558919ab9 in CallableCustomMethodPointer<EditorPropertyArray, double>::call (this=0x555576f0c0b0, p_arguments=0x7fffffffae20, p_argcount=1, r_return_value=..., r_call_error=...) at ./core/object/callable_method_pointer.h:98
#6  0x000055555a53a1ec in Callable::callp (this=this@entry=0x7fff8801fea0, p_arguments=p_arguments@entry=0x7fffffffae20, p_argcount=p_argcount@entry=1, r_return_value=..., r_call_error=...) at core/variant/callable.cpp:57
#7  0x000055555a727e6d in Object::emit_signalp (this=0x555576f09e00, p_name=..., p_args=0x7fffffffae20, p_argcount=1) at core/object/object.cpp:1127
#8  0x00005555592cd39e in Node::emit_signalp (this=0x555576f09e00, p_name=..., p_args=0x7fffffffae20, p_argcount=1) at scene/main/node.cpp:3606
#9  0x000055555810d846 in Object::emit_signal<double> (this=0x555576f09e00, p_name=...) at ./core/object/object.h:922
#10 0x00005555594b2a03 in Range::_value_changed_notify (this=0x555576f09e00) at scene/gui/range.cpp:48
#11 0x00005555594b2a9f in Range::Shared::emit_value_changed (this=<optimized out>) at scene/gui/range.cpp:58
#12 0x00005555594b2b2a in Range::set_value (this=this@entry=0x555576f09e00, p_val=<optimized out>) at scene/gui/range.cpp:92
#13 0x0000555558b188e5 in EditorSpinSlider::gui_input (this=0x555576f09e00, p_event=...) at editor/gui/editor_spin_slider.cpp:65
#14 0x00005555593cfcec in Control::_call_gui_input (this=this@entry=0x555576f09e00, p_event=...) at scene/gui/control.cpp:1810
#15 0x000055555930ca7c in Viewport::_gui_call_input (this=this@entry=0x55555f36c330, p_control=<optimized out>, p_input=...) at scene/main/viewport.cpp:1601
#16 0x000055555931a06e in Viewport::_gui_input_event (this=0x55555f36c330, p_event=...) at scene/main/viewport.cpp:1833
#17 0x000055555931e155 in Viewport::push_input (this=0x55555f36c330, p_event=..., p_local_coords=80, p_local_coords@entry=false) at scene/main/viewport.cpp:3334
#18 0x00005555593403fa in Window::_window_input (this=0x55555f36c330, p_ev=...) at scene/main/window.cpp:1567
#19 0x000055555936065a in call_with_variant_args_helper<Window, Ref<InputEvent> const&, 0ul> (p_instance=<optimized out>, p_method=<optimized out>, p_args=<optimized out>, r_error=...) at ./core/variant/binder_common.h:303
#20 0x00005555593606ba in call_with_variant_args<Window, Ref<InputEvent> const&> (p_instance=<optimized out>, p_method=<optimized out>, p_args=p_args@entry=0x7fffffffd3a0, p_argcount=p_argcount@entry=1, r_error=...) at ./core/variant/binder_common.h:417
#21 0x0000555559360759 in CallableCustomMethodPointer<Window, Ref<InputEvent> const&>::call (this=0x55556e27fdf0, p_arguments=0x7fffffffd3a0, p_argcount=1, r_return_value=..., r_call_error=...) at ./core/object/callable_method_pointer.h:98
#22 0x000055555a53a1ec in Callable::callp (this=this@entry=0x7fffffffd430, p_arguments=p_arguments@entry=0x7fffffffd3a0, p_argcount=p_argcount@entry=1, r_return_value=..., r_call_error=...) at core/variant/callable.cpp:57
#23 0x00005555577bf964 in Callable::call<Ref<InputEvent> > (this=this@entry=0x7fffffffd430) at ./core/variant/variant.h:849
#24 0x00005555577b755d in DisplayServerX11::_dispatch_input_event (this=<optimized out>, p_event=...) at platform/linuxbsd/x11/display_server_x11.cpp:4001
#25 0x00005555577b760f in DisplayServerX11::_dispatch_input_events (p_event=...) at platform/linuxbsd/x11/display_server_x11.cpp:3977
#26 0x000055555a4ff4dc in Input::_parse_input_event_impl (this=this@entry=0x55555d604050, p_event=..., p_is_emulated=p_is_emulated@entry=false) at core/input/input.cpp:730
#27 0x000055555a5001eb in Input::flush_buffered_events (this=0x55555d604050) at core/input/input.cpp:994
#28 0x00005555577ba5a1 in DisplayServerX11::process_events (this=0x55555dd33c80) at platform/linuxbsd/x11/display_server_x11.cpp:5071
#29 0x00005555577992bf in OS_LinuxBSD::run (this=this@entry=0x7fffffffd7d0) at platform/linuxbsd/os_linuxbsd.cpp:929
#30 0x0000555557797d02 in main (argc=<optimized out>, argv=0x7fffffffdda8) at platform/linuxbsd/godot_linuxbsd.cpp:74

[limbonaut]

Are you using some specific build options that I could try to reproduce the issue?
Mine are: platform=linuxbsd target=editor linker=mold dev_build=yes debug_symbols=yes tests=yes

[Rubonnek]

I just finished bisecting again and now it looks like godotengine/godot@db71754 is causing the crash.

I'm using:

scons -Q -s platform=linuxbsd dev_mode=yes dev_build=yes udev=yes target=editor debug_symbols=yes precision=single bits=64 optimize=debug compiledb=yes linker=gold tests=yes werror=no -j$(nproc)

[limbonaut]

So it means that instead of deferred-adding property editor as a child, it's now added immediately. That might cause the object and property to be set after the notification() fires. Maybe, the initialization code from THEME_CHANGED could be delayed enough for the object pointer to be set.

[limbonaut]

I just finished bisecting again and now it looks like godotengine/godot@db71754 is causing the crash.

I'm using:

scons -Q -s platform=linuxbsd dev_mode=yes dev_build=yes udev=yes target=editor debug_symbols=yes precision=single bits=64 optimize=debug compiledb=yes linker=gold tests=yes werror=no -j$(nproc)

Yeah, I think you found the real culprit. I looked at the initialization code in THEME_CHANGED and _setup and I think it needs to go to update_property or be delayed.

[Rubonnek]

I looked at the initialization code in THEME_CHANGED and _setup and I think it needs to go to update_property or be delayed.

Makes sense. I did a rough test by removing add_property_editor from here:

limboai/editor/editor_property_bb_param.cpp

Line 386 in b220a9f

add_property_editor(p_path, editor);

And I don't get a crash anymore.

I'll follow up tomorrow on this.