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

Crash when hiding a panel from a NOTIFICATION_MOUSE_ENTER notification #85277

Closed
Efimero opened this issue Nov 23, 2023 · 3 comments · Fixed by #85284 or #85313
Closed

Crash when hiding a panel from a NOTIFICATION_MOUSE_ENTER notification #85277

Efimero opened this issue Nov 23, 2023 · 3 comments · Fixed by #85284 or #85313

Comments

@Efimero
Copy link

Efimero commented Nov 23, 2023

Godot version

v4.2.rc.custom_build [7022271]

System information

Godot v4.2.rc (7022271) - Debian GNU/Linux trixie/sid trixie - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 6GB (nvidia) - AMD Ryzen 5 1600 Six-Core Processor (12 Threads)

Issue description

Found a consistent crash when calling hide() from _notification().
It does not happen if the call is deferred.

Steps to reproduce

The hide_test.gd script has a boolean cause_crash. If it is true, hovering the mouse over the label crashes without an error in the Godot Editor Console. If it's false it hides the label as expected.

Minimal reproduction project

hide_crash.zip

@YuriSizov YuriSizov changed the title libc crash when hiding a panel Crash when hiding a panel from a NOTIFICATION_MOUSE_ENTER notification Nov 23, 2023
@akien-mga akien-mga added this to the 4.2 milestone Nov 23, 2023
@akien-mga
Copy link
Member

I confirm the crash, and it's a regression in 4.2.

Up to 4.2-dev2 included, the MRP works fine, like in 4.1.3-stable.

From 4.2-dev3 to 4.2-beta5, the MRP doesn't crash, but the label isn't hidden. The NOTIFICATION_MOUSE_ENTER isn't received on enter.

From 4.2-beta6 onward, it's crashing as described in the OP, with this backtrace:

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.2.rc.custom_build (bb63963486a389d4417ba13249a40219f3774df0)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib64/libc.so.6(+0x36960) [0x7efc38487960] (??:0)
[2] Object::notification(int, bool) (/home/akien/Projects/godot/godot.git/./core/object/object.cpp:837)
[3] Viewport::_update_mouse_over(Vector2) (/home/akien/Projects/godot/godot.git/./scene/main/viewport.cpp:3212)
[4] Window::_update_mouse_over(Vector2) (/home/akien/Projects/godot/godot.git/./scene/main/window.cpp:2633)
[5] Viewport::_update_mouse_over() (/home/akien/Projects/godot/godot.git/./scene/main/viewport.cpp:3089)
[6] Viewport::push_input(Ref<InputEvent> const&, bool) (/home/akien/Projects/godot/godot.git/./scene/main/viewport.cpp:3308)
[7] Window::_window_input(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/./scene/main/window.cpp:1551)
[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/Projects/godot/godot.git/./core/variant/binder_common.h:303 (discriminator 4))
[9] void call_with_variant_args<Window, Ref<InputEvent> const&>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, int, Callable::CallError&) (/home/akien/Projects/godot/godot.git/./core/variant/binder_common.h:418)
[10] CallableCustomMethodPointer<Window, Ref<InputEvent> const&>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Projects/godot/godot.git/./core/object/callable_method_pointer.h:105)
[11] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Projects/godot/godot.git/./core/variant/callable.cpp:57)
[12] Variant Callable::call<Ref<InputEvent> >(Ref<InputEvent>) const (/home/akien/Projects/godot/godot.git/./core/variant/variant.h:850)
[13] DisplayServerX11::_dispatch_input_event(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/platform/linuxbsd/x11/display_server_x11.cpp:4001)
[14] DisplayServerX11::_dispatch_input_events(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/platform/linuxbsd/x11/display_server_x11.cpp:3978)
[15] Input::_parse_input_event_impl(Ref<InputEvent> const&, bool) (/home/akien/Projects/godot/godot.git/./core/input/input.cpp:731)
[16] Input::flush_buffered_events() (/home/akien/Projects/godot/godot.git/./core/input/input.cpp:995)
[17] DisplayServerX11::process_events() (/home/akien/Projects/godot/godot.git/platform/linuxbsd/x11/display_server_x11.cpp:5072)
[18] OS_LinuxBSD::run() (/home/akien/Projects/godot/godot.git/platform/linuxbsd/os_linuxbsd.cpp:931)
[19] godot-git(main+0x15a) [0x570bc00] (/home/akien/Projects/godot/godot.git/platform/linuxbsd/godot_linuxbsd.cpp:76)
[20] /lib64/libc.so.6(+0x236b7) [0x7efc384746b7] (??:0)
[21] /lib64/libc.so.6(__libc_start_main+0x85) [0x7efc38474775] (??:0)
[22] godot-git(_start+0x21) [0x570b9e1] (??:?)
-- END OF BACKTRACE --
================================================================

The regression timings are consistent with this being a consequence of #67791 (which regressed on the signal emission) and #84547 (which fixed the regression, but now we get a crash instead).

CC @Sauermann @kitbdev

@kitbdev
Copy link
Contributor

kitbdev commented Nov 24, 2023

A related crash can still happen on mouse exit when hiding a grandparent.
Here's an MRP:
hide_crash2.zip
In this case an index goes out of bounds in Viewport::_drop_mouse_over() line 3273 .

ERROR: FATAL: Index p_index = 0 is out of bounds (count = 0).
   at: LocalVector<class Control *,unsigned int,0,0>::operator [] (C:\Projects\godotsrc\godot\core/templates/local_vector.h:161)
C:\Projects\godotsrc\godot\core/templates/local_vector.h:161 - FATAL: Index p_index = 0 is out of bounds (count = 0).

We should probably check if the gui.mouse_over_hierarchy has a different size than expected and return out if that happens. Or we could have a flag to check if it was modified to prevent _drop_mouse_over from being called from itself, or call the function deferred.
It's also possible it happens in Viewport::_gui_update_mouse_over() line 2517 so we should check that loop as well.

Also Mouse enter and exit notifications may trigger strangely if hide() is called from them.

@akien-mga akien-mga reopened this Nov 24, 2023
@kitbdev
Copy link
Contributor

kitbdev commented Nov 24, 2023

Here is an updated mrp that shows the crash happening in Viewport::_gui_update_mouse_over() using set mouse filter.
filter_crash.zip
Run filter_crash.tscn, move the mouse over the panel and hit space. Make sure to not mouse over the top left corner, some invisible Controls are there. Also make sure not to mouse exit the panel before hitting space.

Hitting space will change the panels filter from pass to stop, which sends mouse exit notifications to its parents.
When the panel's parent receives mouse exit, it modifies the mouse over hierarchy by setting another Control's mouse filter to ignore.
Changing the mouse over hierarchy like this during an exit notification causes index out of bounds.
Setting top_level to true instead of changing the mouse filter will also trigger the crash, since they use the same code.

BTW, This won't happen for mouse enter notification because a list of Controls is used instead of a list of indexes, so it can't go out of bounds.

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