Godot 4.3: crash when using ControllerIconTexture as external resource file on macOS

[mynameiswhm]

Context: I'm running Godot 4.3 (official build, also tried on a build with debug symbols) on M1 Pro (macOS 14.5) with two keyboard layouts installed.

I've tried migrating my existing project to 4.3 and encountered an editor crash, which I've pinpointed to a single ControllerIconTexture saved as .tres file (which I use in a RichTextLabel). The editor starts to load, produces some Generated 'res://%resourcename%' preview in N usec entries, and then crashes.

I have many instances of ControllerIconTexture used as built-in scene resources, and the crash only happens with the external resource.

Here's the resource file:

[gd_resource type="Texture2D" script_class="ControllerIconTexture" load_steps=2 format=3 uid="uid://ckkmayetgcbwl"]

[ext_resource type="Script" path="res://addons/controller_icons/objects/ControllerIconTexture.gd" id="1_kjf73"]

[resource]
resource_local_to_scene = false
resource_name = ""
script = ExtResource("1_kjf73")
path = "special"
show_mode = 0
force_type = 0

The special action is currently mapped to physical Shift, but the particular key doesn't seem to matter much.
If I delete either this single resource file or the special keyboard key from the Input Map (and leave the controller key, for example), everything works fine.

After looking at the backtrace, I've found that the problem happens on this line in the call to DisplayServer.keyboard_get_keycode_from_physical():

controller_icons/addons/controller_icons/ControllerIcons.gd

Line 364 in a102147

return _convert_key_to_path(DisplayServer.keyboard_get_keycode_from_physical(event.physical_keycode))

If I comment this call and use something like return _convert_key_to_path(event.physical_keycode), the crash disappears.

I'm unable to reproduce this crash on a smaller scale yet. Since the crash started happening after migration to 4.3, it's probably best to also report it to the engine maintainers, but I'm lacking the needed context right now. @rsubtil maybe you would have an idea about the root cause?

Here's the crash log from the editor:

================================================================
handle_crash: Program crashed with signal 5
Engine version: Godot Engine v4.3.stable.official (77dcf97d82cbfe4e4615475fa52ca03da645dbd8)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] 1   libsystem_platform.dylib            0x000000018d71b584 _sigtramp + 56
[2] 2   ???                                 0xffff80018d53c820 0x0 + 18446603342887241760
[3] 3   libdispatch.dylib                   0x000000018d53c7b0 _dispatch_assert_queue_fail + 0
[4] 4   HIToolbox                           0x0000000197f6e860 islGetInputSourceListWithAdditions + 160
[5] 5   HIToolbox                           0x0000000197f70e9c isValidateInputSourceRef + 92
[6] 6   HIToolbox                           0x0000000197f70d5c TSMGetInputSourceProperty + 44
[7] KeyMappingMacOS::remap_key(unsigned int, unsigned int, bool) (in Godot) + 108
[8] DisplayServerMacOS::keyboard_get_keycode_from_physical(Key) const (in Godot) + 64
[9] DisplayServerHeadless::_dispatch_input_events(Ref<InputEvent> const&)
[10] DisplayServerHeadless::_dispatch_input_events(Ref<InputEvent> const&)
[11] DisplayServerHeadless::_dispatch_input_events(Ref<InputEvent> const&)
[12] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (in Godot) + 50624
[13] Object::callp(StringName const&, Variant const**, int, Callable::CallError&) (in Godot) + 176
[14] Variant::callp(StringName const&, Variant const**, int, Variant&, Callable::CallError&) (in Godot) + 260
[15] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (in Godot) + 44504
[16] Object::callp(StringName const&, Variant const**, int, Callable::CallError&) (in Godot) + 176
[17] Variant::callp(StringName const&, Variant const**, int, Variant&, Callable::CallError&) (in Godot) + 260
[18] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (in Godot) + 44504
[19] Object::callp(StringName const&, Variant const**, int, Callable::CallError&) (in Godot) + 176
[20] Variant::callp(StringName const&, Variant const**, int, Variant&, Callable::CallError&) (in Godot) + 260
[21] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (in Godot) + 44504
[22] Object::callp(StringName const&, Variant const**, int, Callable::CallError&) (in Godot) + 176
[23] Variant::callp(StringName const&, Variant const**, int, Variant&, Callable::CallError&) (in Godot) + 260
[24] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (in Godot) + 44504
[25] Object::callp(StringName const&, Variant const**, int, Callable::CallError&) (in Godot) + 176
[26] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (in Godot) + 228
[27] CallQueue::_call_function(Callable const&, Variant const*, int, bool) (in Godot) + 296
[28] CallQueue::flush() (in Godot) + 352
[29] ResourceLoader::_thread_load_function(void*) (in Godot) + 700
[30] ResourceLoader::_load_start(String const&, String const&, ResourceLoader::LoadThreadMode, ResourceFormatLoader::CacheMode) (in Godot) + 1608
[31] ResourceLoader::load(String const&, String const&, ResourceFormatLoader::CacheMode, Error*) (in Godot) + 84
[32] EditorResourcePreviewGenerator::generate_from_path(String const&, Vector2 const&, Dictionary&) const (in Godot) + 1292
[33] EditorResourcePreview::_generate_preview(Ref<ImageTexture>&, Ref<ImageTexture>&, EditorResourcePreview::QueueItem const&, String const&, Dictionary&) (in Godot) + 1688
[34] EditorResourcePreview::_iterate() (in Godot) + 1580
[35] EditorResourcePreview::_thread() (in Godot) + 92
[36] Thread::callback(unsigned long long, Thread::Settings const&, void (*)(void*), void*) (in Godot) + 120
[37] Thread::~Thread()
[38] 38  libsystem_pthread.dylib             0x000000018d6eaf94 _pthread_start + 136
[39] 39  libsystem_pthread.dylib             0x000000018d6e5d34 thread_start + 8
-- END OF BACKTRACE --
================================================================

[mynameiswhm]

Another note: if I recreate the same resource after starting without it (except for new auto-generated IDs, of course), the editor works fine until the next restart. But when I used the "duplicate file" feature, the editor crashed again.

I've also managed to create a small reproducible project by creating two resource files: test-crash.zip (that's only the project without addons folder)
Crashes on macOS every time, but works fine on Windows. Probably some threading issue?

[mynameiswhm]

I've managed to run the editor and the game by commenting _load_texture_path_main_thread.call_deferred() on line 177 — looks like something changed in the engine regarding call_deferred and the main thread 🤔 (probably related to EditorResourcePreview::_thread() from the backtrace above)

controller_icons/addons/controller_icons/objects/ControllerIconTexture.gd

Lines 174 to 179 in a102147

	func _load_texture_path():
	# Ensure loading only occurs on the main thread
	if OS.get_thread_caller_id() != OS.get_main_thread_id():
	_load_texture_path_main_thread.call_deferred()
	else:
	_load_texture_path_main_thread()

[rsubtil]

Thank you for your detailed report! Definitely seems to be an issue regarding multi-threading, since AFAIK resource preview generation on the editor happens on a separate thread.

It's interesting that you mention this occurs with 4.3 now; although probably for different reason, there is a related crash happening on another project on similar circumstances. In that case, the regression was introduced on v4.3.beta1, with the latest working version being v4.3.dev6. Can you test that version (v4.3.dev6) to check if it also stops crashing on your case?

[mynameiswhm]

Yeah, my test case works on v4.3.dev6 and crashes on v4.3.beta1, although with different errors:

Godot(89501,0x16cb77000) malloc: *** error for object 0x600003af8d80: pointer being freed was not allocated
Godot(89501,0x16cb77000) malloc: *** set a breakpoint in malloc_error_break to debug
[1]    89501 abort      ~/Downloads/Godot\ 5.app/Contents/MacOS/Godot

I've also built fixes from this PR from related issue: godotengine/godot#94169
Still crashes on DisplayServerMacOS::keyboard_get_keycode_from_physical :(

I'll try to bisect the problem commit between dev6 and beta1 later today.

[mynameiswhm]

This commit is what started the malloc crashes: godotengine/godot@187e5ef
Now I'm going to find where it changes to the error from original post :)

[mynameiswhm]

Damn, I've just wasted a couple of hours to independently find a reference to the same issues you've already mentioned :D

So the summary is (as far as I understand):

• The materials refactor from this PR Let materials' shaders update happen on loader threads godotengine/godot#91630 moved all relevant operations to a separate thread, while introducing a malloc-related crash.
• This PR [ResourceLoader] Add check to prevent double free crashes. godotengine/godot#95186 fixed the malloc-related crash, but the loading is still happening on a separate thread.

If not being able to run this code on the main thread is the new normal for loading resources, then the fix from this PR should help #96 (I've tried it on my case, works fine)
(Although, to be fair, that's not very intuitive to the end-user of the engine).

I've also tried cherry-picking the two PRs mentioned in this one godotengine/godot#95186 (godotengine/godot#94169, godotengine/godot#93336) on top 4.3-stable — still still crashes :(

[rsubtil]

I've researched a bit more regarding this, and while I initially thought this instance was an engine bug, it doesn't appear to be the case.

Although there's basically no documentation for TISGetInputSourceProperty (the native macOS function Godot calls on DisplayServerMacOS::keyboard_get_keycode_from_physical), it's safe to say this isn't a thread-safe function, judging from your analysis. Which is fine, since the addon shouldn't be making running this on other threads in the first place.

This was handled in the past (#82) by using a call_deferred which had the implicit consequence of having the deferred call be executed on the main thread. But with the mentioned engine changes, that no longer seems to be the case, and so I've changed this to a more robust solution to ensure it is called on the main thread (#100).

Could you give this a try to ensure the crash is now fixed, please?
https://github.com/rsubtil/controller_icons/archive/refs/heads/make_texture_load_thread_safe.zip

[mynameiswhm]

Thanks for the deep dive! This version works great in my case!