Godot: Godot crashes when removing file in UI

[spencewenski]

Godot version:
3.2.beta4.mono.official

OS/device including version:
Windows 10 Pro version 1903 build 18362.535

Issue description:
When I remove a file from the Godot UI, Godot crashes about 50% of the time.

Steps to reproduce:

1. Open the example project.
2. Remove the Test.gd and/or Test.tscn file ('Delete' key or 'Right Click > Delete') using the Godot UI
3. Godot crashes

Repro rate
~50%

Note
Removing the files outside of Godot works, and Godot's UI updates as expected to no longer show the removed files.

Minimal reproduction project:
RemoveFileCrash.zip

Here are Godot logs from just opening and closing the project: OpenAndCloseProject - 2019_12_29 15.16.20 (13604).txt

And here are logs of removing a file and Godot crashing: OpenAndRemoveFile - 2019_12_29 15.17.23 (12400).txt

Also here's my editor settings (extension changed to upload to github): editor_settings-3.txt

[dankan1890]

It is a regression between beta3 and beta4.

[akien-mga]

Does this happen with the non-Mono build? If so, what's the crash log from running it in a terminal?

[dankan1890]

It also happens with a build without Mono:

Godot Engine v3.2.beta.custom_build.27b74f42b - https://godotengine.org
OpenGL ES 3.0 Renderer: Radeon RX 560 Series

* daemon started successfully
erasing C:/Users/Asus/Documents/Godot Projects/Test Project/.import/icon_audio_bus_bypass.png-f7c706b30d4671452bc837c9345a5ed8.stex
erasing C:/Users/Asus/Documents/Godot Projects/Test Project/icon_audio_bus_bypass.png.import
CrashHandlerException: Program crashed
Dumping the backtrace. Please include this when reporting the bug on 
https://github.com/godotengine/godot/issues
[0] Viewport::_make_input_local (C:\Users\Asus\Documents\Godot-Source\godot\scene\main\viewport.cpp:1383)
[1] Viewport::_vp_unhandled_input (C:\Users\Asus\Documents\Godot-Source\godot\scene\main\viewport.cpp:1435)
[2] MethodBind1<Viewport,Ref<InputEvent> const &>::call (C:\Users\Asus\Documents\Godot-Source\godot\core\method_bind.gen.inc:867)
[3] Object::call (C:\Users\Asus\Documents\Godot-Source\godot\core\object.cpp:921)
[4] Object::call (C:\Users\Asus\Documents\Godot-Source\godot\core\object.cpp:847)
[5] SceneTree::call_group_flags (C:\Users\Asus\Documents\Godot-Source\godot\scene\main\scene_tree.cpp:275)
[6] SceneTree::input_event (C:\Users\Asus\Documents\Godot-Source\godot\scene\main\scene_tree.cpp:447)
[7] InputDefault::_parse_input_event_impl (C:\Users\Asus\Documents\Godot-Source\godot\main\input_default.cpp:442)
[8] InputDefault::parse_input_event (C:\Users\Asus\Documents\Godot-Source\godot\main\input_default.cpp:260)
[9] InputDefault::flush_accumulated_events (C:\Users\Asus\Documents\Godot-Source\godot\main\input_default.cpp:679)
[10] OS_Windows::process_events (C:\Users\Asus\Documents\Godot-Source\godot\platform\windows\os_windows.cpp:2445)
[11] OS_Windows::run (C:\Users\Asus\Documents\Godot-Source\godot\platform\windows\os_windows.cpp:3140)
[12] widechar_main (C:\Users\Asus\Documents\Godot-Source\godot\platform\windows\godot_windows.cpp:162)
[13] _main (C:\Users\Asus\Documents\Godot-Source\godot\platform\windows\godot_windows.cpp:184)
[14] main (C:\Users\Asus\Documents\Godot-Source\godot\platform\windows\godot_windows.cpp:196)
[15] __scrt_common_main_seh (d:\agent\_work\5\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[16] BaseThreadInitThunk
-- END OF BACKTRACE --

Probably related to this change in scene_tree:
https://github.com/godotengine/godot/pull/34040/files#diff-ab5997b3cc868f482274b0e0ec449b4d

[dankan1890]

Further information:
the crash happens for every operation that involves a cancellation of a file (ex: export to an existing file) confirming the last dialog with the keyboard (Enter) and not with the mouse.

[ericrybick]

Probably related to this change in scene_tree:
https://github.com/godotengine/godot/pull/34040/files#diff-ab5997b3cc868f482274b0e0ec449b4d

I can confirm that reverting the line in scene_tree.cpp fixes the crash.

[akien-mga]

I can't seem to reproduce the crash in OP with a debug build of the master branch (0aefec2), but I can reproduce it with exporting to an existing file as described in https://github.com/godotengine/godot/issues/34691#issuecomment-570932384

(gdb) bt
#0  0x00007ffff7b432f7 in __dynamic_cast () from /lib64/libstdc++.so.6
#1  0x00000000014488dc in Object::cast_to<Reference> (p_object=0x700000006a) at ./core/object.h:601
#2  0x00000000021b3746 in Ref<Reference>::operator=<InputEvent> (this=0x7fffffffc658, p_from=...) at ./core/reference.h:163
#3  0x00000000021b0ab8 in Ref<InputEvent>::get_ref_ptr (this=0xb0ffbf0) at ./core/reference.h:140
#4  0x00000000021ae7a8 in Ref<InputEvent>::operator Variant (this=0xb0ffbf0) at ./core/reference.h:146
#5  0x0000000002ab574e in SceneTree::input_event (this=0x64033e0, p_event=...) at scene/main/scene_tree.cpp:447
#6  0x0000000001454fb4 in InputDefault::_parse_input_event_impl (this=0x554dee0, p_event=..., p_is_emulated=false) at main/input_default.cpp:442
#7  0x0000000001453e62 in InputDefault::parse_input_event (this=0x554dee0, p_event=...) at main/input_default.cpp:259
#8  0x0000000001456132 in InputDefault::flush_accumulated_events (this=0x554dee0) at main/input_default.cpp:678
#9  0x0000000001440980 in OS_X11::process_xevents (this=0x7fffffffcf70) at platform/x11/os_x11.cpp:2694
#10 0x0000000001443cf5 in OS_X11::run (this=0x7fffffffcf70) at platform/x11/os_x11.cpp:3258
#11 0x00000000014341cc in main (argc=2, argv=0x7fffffffd738) at platform/x11/godot_x11.cpp:56

We can definitely change that line back to what it was if it causes crashes, though it would be good to understand why having it a const reference creates problems.

[ericrybick]

I think I found the cause of the crashing. (on windows) - I wasn't able to reproduce the crashes on linux though. Neither by deleting with the return key, nor by exporting to an existing file. So I can't say anything regarding that.

So basically what happens is that move_to_trash() calls release_pressed_events() afterwards. release_pressed_events() leads to an indirect recursiv call of SceneTree::input_event(), which results in ev being processed twice in that function. However, it gets deleted by InputDefault::flush_accumulated_events() after the first loop. This causes the crashing.

This all sounds a bit complicated, that's why I created this little flow chart (I hope it helps):