godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.67k stars 20.11k forks source link

Using `InputMap.ActionGetEvents` with StringName from `InputMap.GetActions` causes a crash in Godot 4.2.2 mono #89941

Closed hhyyrylainen closed 2 weeks ago

hhyyrylainen commented 5 months ago

Tested versions

Update: also reproducible now in the 4.2.2.mono full release.

System information

Fedora 39, probably not related to rendering or GPU

Issue description

Calling the C# Godot API InputMap.ActionGetEvents() with a StringName from InputMap.GetActions() causes the following crash on shutdown:

Thread 1 "godot.linuxbsd." received signal SIGSEGV, Segmentation fault.
0x000000000433041f in __dynamic_cast ()
(gdb) bt
#0  0x000000000433041f in __dynamic_cast ()
#1  0x0000000000b72ec2 in Object::cast_to<RefCounted> (p_object=<optimized out>) at ./core/object/object.h:791
#2  CSharpLanguage::_instance_binding_reference_callback (p_token=<optimized out>, p_binding=0x99b0ce0, p_reference=0 '\000')
    at modules/mono/csharp_script.cpp:1376
#3  0x000000000403bcc2 in Object::_instance_binding_reference (p_reference=false, this=0x6c114e0) at ./core/object/object.h:678
#4  RefCounted::unreference (this=0x6c114e0) at core/object/ref_counted.cpp:89
#5  0x0000000001635b94 in Ref<InputEvent>::unref (this=0x6c68590) at ./core/object/ref_counted.h:209
#6  Ref<InputEvent>::~Ref (this=0x6c68590, __in_chrg=<optimized out>) at ./core/object/ref_counted.h:222
#7  List<Ref<InputEvent>, DefaultAllocator>::Element::~Element (this=0x6c68590, __in_chrg=<optimized out>) at ./core/templates/list.h:51
#8  memdelete_allocator<List<Ref<InputEvent>, DefaultAllocator>::Element, DefaultAllocator> (p_class=0x6c68590) at ./core/os/memory.h:121
#9  List<Ref<InputEvent>, DefaultAllocator>::_Data::erase (p_I=0x6c68590, this=<optimized out>) at ./core/templates/list.h:243
#10 List<Ref<InputEvent>, DefaultAllocator>::erase (p_I=0x6c68590, this=0x6c684f0) at ./core/templates/list.h:431
#11 List<Ref<InputEvent>, DefaultAllocator>::clear (this=0x6c684f0) at ./core/templates/list.h:464
#12 List<Ref<InputEvent>, DefaultAllocator>::~List (this=0x6c684f0, __in_chrg=<optimized out>) at ./core/templates/list.h:759
#13 0x0000000003d835d1 in InputMap::Action::~Action (this=0x6c684e8, __in_chrg=<optimized out>) at core/input/input_map.h:51
#14 KeyValue<StringName, InputMap::Action>::~KeyValue (this=0x6c684e0, __in_chrg=<optimized out>) at ./core/templates/pair.h:82
#15 HashMapElement<StringName, InputMap::Action>::~HashMapElement (this=0x6c684d0, __in_chrg=<optimized out>) at ./core/templates/hash_map.h:55
#16 memdelete<HashMapElement<StringName, InputMap::Action> > (p_class=0x6c684d0) at ./core/os/memory.h:109
#17 DefaultTypedAllocator<HashMapElement<StringName, InputMap::Action> >::delete_allocation (p_allocation=0x6c684d0, this=0x6b2c000)
    at ./core/os/memory.h:206
#18 HashMap<StringName, InputMap::Action, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, InputMap::Action> > >::clear (this=0x6b2c000) at ./core/templates/hash_map.h:265
#19 HashMap<StringName, InputMap::Action, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, InputMap::Action> > >::clear (this=0x6b2c000) at ./core/templates/hash_map.h:254
#20 HashMap<StringName, InputMap::Action, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, InputMap::Action> > >::~HashMap (this=0x6b2c000, __in_chrg=<optimized out>) at ./core/templates/hash_map.h:616
#21 InputMap::~InputMap (this=0x6b2be90, __in_chrg=<optimized out>) at core/input/input_map.cpp:815
#22 0x000000000049d620 in memdelete<InputMap> (p_class=0x6b2be90) at ./core/os/memory.h:104
#23 memdelete<InputMap> (p_class=0x6b2be90) at ./core/os/memory.h:104
#24 Main::cleanup (p_force=p_force@entry=false) at main/main.cpp:3887
#25 0x00000000004256db in main (argc=<optimized out>, argv=0x7fffffffdbe8) at platform/linuxbsd/godot_linuxbsd.cpp:76

The more times I call InputMap.ActionGetEvents the more likely the crash is. Just accessing one value doesn't cause a crash very often, but already at 2 I get pretty regular crashes. With 3 I get almost always a crash. The sample code I set it to take 10 items which seems to always crash in a debugger. Using a value over 50 seems to have a good chance to crash even when not using a debugger.

Also if I use the following code to not reuse the StringName instance, the crash goes away (note the GC.Collect is required to not cause a crash):

        string name;
        {
            var list = InputMap.GetActions().Take(10).ToList();
            name = list[0].ToString();
        }

        // This line is important
        GC.Collect();

        foreach (var inputEvent in InputMap.ActionGetEvents(name));

With fewer StringNames used the crash is less consistent and sometimes I've observed these other places where the crash happens:

ERROR: FATAL: Condition "!rc_owner" is true.
   at: _instance_binding_reference_callback (modules/mono/csharp_script.cpp:1379)

Thread 1 "godot.linuxbsd." received signal SIGILL, Illegal instruction.
0x0000000000b72fbc in CSharpLanguage::_instance_binding_reference_callback (p_token=<optimized out>, p_binding=<optimized out>, p_reference=<optimized out>)
    at modules/mono/csharp_script.cpp:1379
1379        CRASH_COND(!rc_owner);
(gdb) bt
#0  0x0000000000b72fbc in CSharpLanguage::_instance_binding_reference_callback (p_token=<optimized out>, p_binding=<optimized out>, 
    p_reference=<optimized out>) at modules/mono/csharp_script.cpp:1379
#1  0x000000000403bcc2 in Object::_instance_binding_reference (p_reference=false, this=0x6b75840) at ./core/object/object.h:678
#2  RefCounted::unreference (this=0x6b75840) at core/object/ref_counted.cpp:89
#3  0x0000000001635b94 in Ref<InputEvent>::unref (this=0x6c30640) at ./core/object/ref_counted.h:209
#4  Ref<InputEvent>::~Ref (this=0x6c30640, __in_chrg=<optimized out>) at ./core/object/ref_counted.h:222
#5  List<Ref<InputEvent>, DefaultAllocator>::Element::~Element (this=0x6c30640, __in_chrg=<optimized out>) at ./core/templates/list.h:51
#6  memdelete_allocator<List<Ref<InputEvent>, DefaultAllocator>::Element, DefaultAllocator> (p_class=0x6c30640) at ./core/os/memory.h:121
#7  List<Ref<InputEvent>, DefaultAllocator>::_Data::erase (p_I=0x6c30640, this=<optimized out>) at ./core/templates/list.h:243
#8  List<Ref<InputEvent>, DefaultAllocator>::erase (p_I=0x6c30640, this=0x6c2f7b0) at ./core/templates/list.h:431
#9  List<Ref<InputEvent>, DefaultAllocator>::clear (this=0x6c2f7b0) at ./core/templates/list.h:464
#10 List<Ref<InputEvent>, DefaultAllocator>::~List (this=0x6c2f7b0, __in_chrg=<optimized out>) at ./core/templates/list.h:759
#11 0x0000000003d835d1 in InputMap::Action::~Action (this=0x6c2f7a8, __in_chrg=<optimized out>) at core/input/input_map.h:51
#12 KeyValue<StringName, InputMap::Action>::~KeyValue (this=0x6c2f7a0, __in_chrg=<optimized out>) at ./core/templates/pair.h:82
#13 HashMapElement<StringName, InputMap::Action>::~HashMapElement (this=0x6c2f790, __in_chrg=<optimized out>) at ./core/templates/hash_map.h:55
#14 memdelete<HashMapElement<StringName, InputMap::Action> > (p_class=0x6c2f790) at ./core/os/memory.h:109
#15 DefaultTypedAllocator<HashMapElement<StringName, InputMap::Action> >::delete_allocation (p_allocation=0x6c2f790, this=0x6b2c010)
    at ./core/os/memory.h:206
#16 HashMap<StringName, InputMap::Action, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, InputMap::Action> > >::clear (this=0x6b2c010) at ./core/templates/hash_map.h:265
#17 HashMap<StringName, InputMap::Action, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, InputMap::Action> > >::clear (this=0x6b2c010) at ./core/templates/hash_map.h:254
#18 HashMap<StringName, InputMap::Action, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, InputMap::Action> > >::~HashMap (this=0x6b2c010, __in_chrg=<optimized out>) at ./core/templates/hash_map.h:616
#19 InputMap::~InputMap (this=0x6b2bea0, __in_chrg=<optimized out>) at core/input/input_map.cpp:815
#20 0x000000000049d620 in memdelete<InputMap> (p_class=0x6b2bea0) at ./core/os/memory.h:104
#21 memdelete<InputMap> (p_class=0x6b2bea0) at ./core/os/memory.h:104
#22 Main::cleanup (p_force=p_force@entry=false) at main/main.cpp:3887
#23 0x00000000004256db in main (argc=<optimized out>, argv=0x7fffffffdbe8) at platform/linuxbsd/godot_linuxbsd.cpp:76

And:

Thread 1 "godot.linuxbsd." received signal SIGSEGV, Segmentation fault.
TableFreeSingleHandleToCache (pTable=0x0, uType=0, handle=0x965cd20)
    at /usr/src/debug/dotnet8.0-8.0.102-1.fc39.x86_64/src/runtime/artifacts/source-build/self/src/src/coreclr/gc/handletablecache.cpp:786
Downloading source file /usr/src/debug/dotnet8.0-8.0.102-1.fc39.x86_64/src/runtime/artifacts/source-build/self/src/src/coreclr/gc/handletablecache.cpp
786         if (TypeHasUserData(pTable, uType))                                                                                                               
(gdb) bt
#0  TableFreeSingleHandleToCache (pTable=0x0, uType=0, handle=0x965cd20)
    at /usr/src/debug/dotnet8.0-8.0.102-1.fc39.x86_64/src/runtime/artifacts/source-build/self/src/src/coreclr/gc/handletablecache.cpp:786
#1  0x00007fffa27fc228 in HndDestroyHandle (hTable=0x0, uType=0, handle=0x965cd20)
    at /usr/src/debug/dotnet8.0-8.0.102-1.fc39.x86_64/src/runtime/artifacts/source-build/self/src/src/coreclr/gc/handletable.cpp:383
#2  0x00007fff236b61ce in ?? ()
#3  0x00007fffffffd430 in ?? ()
#4  0x00007fff24165716 in ?? ()
#5  0x00007fffffffd3e0 in ?? ()
#6  0x0000000000000000 in ?? ()

Steps to reproduce

A more tweakable example of the code causing a crash is this:

    // Running this causes a crash on shutdown (when running outside GDB or C# debugger,
    // increase the take variable to see the crash consistently)
    public override void _Ready()
    {
        // It doesn't seem to matter which actions are inspected, just how many are accessed
        // This should be enough to always see the crash. Note that with very low take counts
        // different crashes can be observed, this always gives a SIGSEGV in the same place.
        int take = 10;

        // For crashing without a debugger attached, it is better to use way bigger value
        // int take = 100;

        // This is the minimum amount I need to take to pretty consistently get a crash
        // int take = 3;
        // This still somewhat consistently crashes
        // int take = 2;

        // I cannot get a crash by just taking one item
        // int take = 1;

        foreach (var name in InputMap.GetActions().Take(take))
        {
            // Just calling InputMap.ActionGetEvents doesn't cause a crash, it needs to be
            // accessed for example with iteration or a .ToList() call
            foreach (var inputEvent in InputMap.ActionGetEvents(name));
        }
    }

Minimal reproduction project (MRP)

GodotTestProject.zip

hhyyrylainen commented 5 months ago

I tried to use the mitigation in my real project, but it did not work. I suspect there is something in general wrong with InputMap.ActionGetEvents, or either InputMap.ActionEraseEvents and InputMap.ActionEraseEvents.

PaulUlanovskij commented 3 months ago

Can not get this issue to work on master branch bdc0316 with Godot.NET.Sdk.4.3.0-dev.6. Even if I put your code into a _Process() and then close the window I get no crash. I use arch btw. Or maybe I am just generally doing something wrong

hhyyrylainen commented 3 months ago

I re-tested with Godot Engine v4.3.dev6.mono.official.64520fe67 and found out that the bug now only happens when directly running the game. So for example if I run ./Godot_v4.3-dev6_mono_linux.x86_64 --path /home/hhyyrylainen/Projects/Thrive/ --editor to open the godot editor and hit play, I don't see the crash on shutdown. But if I run ./Godot_v4.3-dev6_mono_linux.x86_64 --path /home/hhyyrylainen/Projects/Thrive/ (so without the --editor flag) I see the crash still when shutting down:

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.3.dev6.mono.official (64520fe6741d8ec3c55e0c9618d3fadcda949f63)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /usr/lib64/dotnet/shared/Microsoft.NETCore.App/8.0.4/libcoreclr.so(+0x606a55) [0x7fc783406a55] (??:0)
[2] /usr/lib64/dotnet/shared/Microsoft.NETCore.App/8.0.4/libcoreclr.so(+0x606085) [0x7fc783406085] (??:0)
[3] /lib64/libc.so.6(+0x3e9a0) [0x7fc7d83a99a0] (??:0)
[4] ./Godot_v4.3-dev6_mono_linux.x86_64() [0x4a0c6b2] (??:0)
[5] ./Godot_v4.3-dev6_mono_linux.x86_64() [0xb94cc2] (??:0)

So it does look like there's a partial fix for this issue in Godot 4.3

PaulUlanovskij commented 3 months ago

Thanks, got it on a second try. Will inspect it a bit later.