godotengine / godot

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

Crash with deferred texture loading on multi-threaded scene loading #95138

Closed rsubtil closed 2 months ago

rsubtil commented 2 months ago

Tested versions

System information

Godot v4.3.dev (187e5ef25) - Arch Linux #1 SMP PREEMPT_DYNAMIC Sat, 27 Jul 2024 16:49:55 +0000 - X11 - Vulkan (Forward+) - dedicated AMD Radeon RX 6750 XT (RADV NAVI22) () - AMD Ryzen 5 5600G with Radeon Graphics (12 Threads)

Issue description

This issue was observed on the Controller Icons addon, of which I'm the developer. This addon essentially contains a custom Texture2D resource that changes dynamically to display the correct input prompt based on player input. Such resources are usually set on nodes, which are then stored in a scene file.

It was reported that when attempting to use multi threading for loading scenes (https://docs.godotengine.org/en/stable/tutorials/io/background_loading.html), such resources would cause crashes. A fix was suggested where the resource only loads textures if it is on the main thread, and defers the function call if it comes from another thread:

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()

This worked up until v4.3.dev6, but started to cause crashes again from v4.3.beta1. Bisecting this lead to https://github.com/godotengine/godot/pull/91630 (cc @RandomShaper). The crash seems to be quite low-level and hinting at some memory corruption; in my case, it prints malloc(): unsorted double linked list corrupted before segfaulting.

Steps to reproduce

Minimal reproduction project (MRP)

threaded_load_controller_icons.zip

bruvzg commented 2 months ago

Seems to be crashing on this line https://github.com/godotengine/godot/blob/3978628c6cc1227250fc6ed45c8d854d24c30c30/core/io/resource_loader.cpp#L411 (double free), adding the following check seems to fix the crash (and loading is working), but I not sure why it can happen, so there's likely some other issue (but adding this check should be safe):

    if (load_paths_stack) {
        memdelete(load_paths_stack);
        load_paths_stack = nullptr;
    }