Renaming Node Crashes Editor on Linux with i3wm

[f8122dac91]

Godot version

4.0.beta17

System information

Linux.x86_64

Issue description

Renaming a node in the scene tree by double clicking randomly crashes the running editor instance with the following error message:

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.0.beta17.official (c40020513ac8201a449b5ae2eeb58fef0ce0a2a4)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /usr/lib/libc.so.6(+0x389e0) [0x7f729c7cf9e0] (??:0)
[2] /opt/godot/Godot_v4.0-beta17_linux.x86_64() [0x44ef5c1] (??:0)
[3] /opt/godot/Godot_v4.0-beta17_linux.x86_64() [0x4b802d7] (??:0)
[4] /opt/godot/Godot_v4.0-beta17_linux.x86_64() [0x2bc69f1] (??:0)
[5] /opt/godot/Godot_v4.0-beta17_linux.x86_64() [0x4957fc4] (??:0)
[6] /opt/godot/Godot_v4.0-beta17_linux.x86_64() [0xe76ab5] (??:0)
[7] /opt/godot/Godot_v4.0-beta17_linux.x86_64() [0xde8ec2] (??:0)
[8] /usr/lib/libc.so.6(+0x23290) [0x7f729c7ba290] (??:0)
[9] /usr/lib/libc.so.6(__libc_start_main+0x8a) [0x7f729c7ba34a] (??:0)
[10] /opt/godot/Godot_v4.0-beta17_linux.x86_64() [0xe0908e] (??:0)
-- END OF BACKTRACE --
================================================================

The error occurs about 8 out of 10 times.

Steps to reproduce

1. Create a blank project
2. Create a scene
3. Add any root node
4. Rename the root node by double clicking the name in the Scene Tree view

Minimal reproduction project

A blank project with no scene produces this error.

[f8122dac91]

Using "rename" from right mouse click context menu is working fine for me.

[Zireael07]

In this case, likely something to do with mouse handling - summon @Sauermann

[Sauermann]

Actually I have tried this, reading this report.
But I was unable to replicate this issue on Debian X11 Xfce with v4.0.beta.custom_build [0b1d516]. So I am not sure how I can help.

[f8122dac91]

FYI I'm on kernel 6.1.8-arch1-1 on arch, X11 i3wm. I'll post more info about this issue if I come across it.

[Rindbee]

Just keep clicking.

0.mp4

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.0.beta.custom_build (dd65b8fda9ba0b9de379ad71985878bdc7cabc42)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x7fe6fad55520] (??:0)
[2] Object::notification(int, bool) (/opt/godot/godot/core/object/object.cpp:790)
[3] Window::_propagate_window_notification(Node*, int) (/opt/godot/godot/scene/main/window.cpp:583)
[4] Window::_event_callback(DisplayServer::WindowEvent) (/opt/godot/godot/scene/main/window.cpp:616)
[5] void call_with_variant_args_helper<Window, DisplayServer::WindowEvent, 0ul>(Window*, void (Window::*)(DisplayServer::WindowEvent), Variant const**, Callable::CallError&, IndexSequence<0ul>) (/opt/godot/godot/./core/variant/binder_common.h:298)
[6] void call_with_variant_args<Window, DisplayServer::WindowEvent>(Window*, void (Window::*)(DisplayServer::WindowEvent), Variant const**, int, Callable::CallError&) (/opt/godot/godot/./core/variant/binder_common.h:408)
[7] CallableCustomMethodPointer<Window, DisplayServer::WindowEvent>::call(Variant const**, int, Variant&, Callable::CallError&) const (/opt/godot/godot/./core/object/callable_method_pointer.h:105)
[8] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/opt/godot/godot/core/variant/callable.cpp:51)
[9] DisplayServerX11::_send_window_event(DisplayServerX11::WindowData const&, DisplayServer::WindowEvent) (/opt/godot/godot/platform/linuxbsd/x11/display_server_x11.cpp:3566)
[10] DisplayServerX11::process_events() (/opt/godot/godot/platform/linuxbsd/x11/display_server_x11.cpp:4175)
[11] OS_LinuxBSD::run() (/opt/godot/godot/platform/linuxbsd/os_linuxbsd.cpp:876)
[12] /opt/godot/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm(main+0x1fe) [0x56411452a86e] (/opt/godot/godot/platform/linuxbsd/godot_linuxbsd.cpp:73)
[13] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7fe6fad3cd90] (??:0)
[14] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7fe6fad3ce40] (??:0)
[15] /opt/godot/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm(_start+0x25) [0x56411452a5a5] (??:?)
-- END OF BACKTRACE --
================================================================

This may be an i3wm-specific issue.

[Rindbee]

Well, I found a related issue #72763, which can be reproduced on i3wm and Cinnamon.

[geowarin]

Can confirm, this happens if I triple click on a node.
The two first clicks make the node enter rename mode and the third dispatches the WINDOW_EVENT_FOCUS_OUT.

If done too fast it leads to a segfault coming from here.

I'm on i3 as well. This is a recent crash I think. I can try to bisect it.

[geowarin]

9f426498231816ccb9d686b3944373f61312aa24 is the first bad commit
commit 9f426498231816ccb9d686b3944373f61312aa24
Author: bruvzg <[email protected]>
Date:   Wed Feb 1 10:51:03 2023 +0200

    [X11] Fix IME subwindow in the popup not getting input focus.

 platform/linuxbsd/x11/display_server_x11.cpp | 2 +-
 scene/gui/popup_menu.cpp                     | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

So caused by #72497.
It's been there for a while, but has become way easier to trigger recently

[bruvzg]

Can't reproduce on i3.

The two first clicks make the node enter rename mode and the third dispatches the WINDOW_EVENT_FOCUS_OUT.

I guess it can happen if the third click arrives before the rename popup window is opened, so it's probably extremely sensitive to timing. But I'm not sure what can cause a crash in the notification.

[geowarin]

While git bisecting, I was reproducing it 100% of the time by clicking a lot on a node.
On latest git revision, 3 clicks are sufficient, in previous revisions I had to click a bunch but this is something I can pull of very consistently in less than 2 seconds.

It IS sensitive to timing.

[geowarin]

2023-02-07.13-41-44.mp4

[geowarin]

It looks like a very tricky one @bruvzg so tell me if there is anything I can do to help

[geowarin]

Also notice how the rename window has an incorrect size/placement on i3 since 9f42649

I'm not getting the crash on gnome.
But within a gnome VM, within i3, my whole VM freezes, similar to what #72012 describes

EDIT: I'm wondering if this can happen in other environments as well, except it is pretty much impossible to click again on the node if the rename windows is placed correctly.

[geowarin]

So I'm going to be blunt.
In its current state, the editor is really easy to crash. I have to maintain my own build for it to be usable by reverting #72370 #72497 #72785 and #72826.

I don't want to be disrespectful to @bruvzg who probably spent an enormous amount of time debugging the X11 code (which is quite messy). I salute you :)

But, the editor was working perfectly before for me, and I'm not sure what #72370 was trying to fix in the first place? IME dead keys? What is it? They were no issues nor proposals associated with it.

If somebody can explain why the original PR was made, what issue it solves and why it was solved this way, it might give somebody a chance to debug this.

Otherwise I suggest just reverting the four commits I mentioned (which are, in reality 1 commit and 3 subsequent bugfixes) because they led to numerous issues on X11 and wayland.
Some of which are crashes and infinite loops.

[Zireael07]

@geowarin If you search for "dead keys" you'll see quite a lot of issues.

My understanding is that #18020 broke people's ability to use dead keys (which are quite common, e.g. the Spanish and International US layouts use them). 72370 makes it possible to use those layouts again (because without it, only IME input works, which is to mean, Chinese/Japanese dead keys, but not Western dead keys because they don't rely on IME)

[geowarin]

Ok thanks @Zireael07, that explains the "why?"

But what does IME has to do with popups? What is the "IME window"? Why is it leaking into Godot's code?
Wasn't the code related to key inputs sufficient to fix the dead key issue?

[bruvzg]

But what does IME has to do with popups?

IME require an invisible sub-window to work (which determine candidate window drawing position, and dispatching input event to the IME engine). So each time a LineEdit or TextEdit is focused, the IME window is mapped and input focus is transferred to it. IME engines and WMs (especially on XWayland) are quite inconsistent in handling it and sensitive to window focus. Before IME support was implementation, all popups (including the node rename popup) were unfocusable, but this does not work on XWayland (it locks all input when focus is transferred to IME sub-window), so it was changed to be focusable. Dead keys without IME (input key handling changes) are not directly related to the issue or IME implementation, it was never supported before and was found only during IME issues debugging.

[geowarin]

Thanks @bruvzg!

So #72370 tries to fix two problems at once:

• Dead keys without IME (which seems to work fine now)
• some IME window focus problems (again, I found no issues talking about this).

Are those IME window focus problems only appearing on XWayland?
Is it possible to debug this with a non Japanese/Chinese keyboard?

[bruvzg]

Are those IME window focus problems only appearing on XWayland?

KDE on XWayland in particular, see #72074 (this is not the only issue, there were some other as well).

Is it possible to debug this with a non Japanese/Chinese keyboard?

No, all keyboards are physically the same. You need to enable IME (usually ibus or fcitx) and add any of the IME enabled keyboard layout (IME setup might be as easy as adding it in the keyboard settings, or running im-config, or require some manual config file editing, depending on distro).

[geowarin]

Ok I'll try. I found an article on arch wiki, BTW

Maybe @bruvzg, you are not able to reproduce the issue because you have IME enabled, which steals the focus thus preventing the "triple click crash".

Are you able to see the weird looking rename popup on i3?

[bruvzg]

you are not able to reproduce the issue because you have IME enabled

No, it's the same with or without IME, all modern distros should have IME enabled (even if you do not have keyboard layout configured) by default. Note: If IME is fully disabled, Godot will print XOpenIM failed warning on the start, so you can detect it this way.

Are you able to see the weird looking rename popup on i3?

Yes, rename popup are too big on i3. But no matter what I try, the third click always lands inside a popup and cause text selection, not popup closing.

The only thing I suspect can have some effect is event callback not being cleaned. Try adding the following code to the Window::_clear_window() in the scene/main/window.cpp:

void Window::_clear_window() {
	ERR_FAIL_COND(window_id == DisplayServer::INVALID_WINDOW_ID);

+	DisplayServer::get_singleton()->window_set_rect_changed_callback(Callable(), window_id);
+	DisplayServer::get_singleton()->window_set_window_event_callback(Callable(), window_id);
+	DisplayServer::get_singleton()->window_set_input_event_callback(Callable(), window_id);
+	DisplayServer::get_singleton()->window_set_input_text_callback(Callable(), window_id);
+	DisplayServer::get_singleton()->window_set_drop_files_callback(Callable(), window_id);
+
	if (transient_parent && transient_parent->window_id != DisplayServer::INVALID_WINDOW_ID) {
		DisplayServer::get_singleton()->window_set_transient(window_id, DisplayServer::INVALID_WINDOW_ID);
	}

[geowarin]

The code you provided does fix the problem for me! Amazing 😄

I have no XOpenIM failed warning on start.

[bruvzg]

The code you provided does fix the problem for me! Amazing

Great, I have opened PR - #73239.