Crash on PackedScene.Instantiate() C# Multithreading

[Targma]

Godot version

4.2-beta1_mono_win64

System information

Windows 10 - Godot_v4.2-beta1_mono_win64 - Forward plus -dedicated

Issue description

C# Non-Main thread crash when calling instantiate Node2D with Label (Control node) Works normally when done with empty Node2d without Label and Sprite2D instead.

Steps to reproduce

Calling

var scene = GD.Load<PackedScene>()
var node = scene.Instantiate()
AddChild(node);

crashes on non-main thread. Call work first time but then crashes when called again

var node2 = scene.Instantiate()'
AddChild(node2);

Full code

Main.cs

public partial class Main : Node2D
{
    public const bool InstantiateOnMainThread = false;
    private static readonly Vector2 InitialPosition = new(250, 250);

    private PackedScene _packedScene;
    private double Timer { get; set; } = 2f;

    // Called when the node enters the scene tree for the first time.
    public override void _Ready()
    {
        _packedScene = GD.Load<PackedScene>("res://NumberLabel.tscn");
    }

    // Called every frame. 'delta' is the elapsed time since the previous frame.
    public override void _Process(double delta)
    {
        Timer -= delta;
        if (Timer > 0) return;
        Timer = 2f;

        if (InstantiateOnMainThread)
        {
            var node = _packedScene.Instantiate<NumberLabel>();
            node.Position = InitialPosition;
            AddChild(node);
        }
        else
        {
            Task.Run(() =>
            {
                var node = _packedScene.Instantiate<NumberLabel>();
                node.Position = InitialPosition;
                CallDeferred(Node.MethodName.AddChild, node);
            }); 
        }
    }
}

Number.cs

public partial class NumberLabel : Node2D
{
    private static readonly Vector2 Movement = new(0, -120);

    private Label Label { get; set; }
    private double Duration { get; set; } = 3f;

    // Called when the node enters the scene tree for the first time.
    public override void _Ready()
    {
        Label = GetNode<Label>("%Label");
    }

    // Called every frame. 'delta' is the elapsed time since the previous frame.
    public override void _Process(double delta)
    {
        Duration -= delta;
        Position += Movement * (float) delta;

        if (Duration > 0) return;
        QueueFree();
    }
}

Minimal reproduction project

Link to project https://github.com/Targma/Godot-Crash-Replication

Modify to test different scenarios

public const bool InstantiateOnMainThread = false;
public const bool SkipLabel = false;

Result when instantiated on main thread and crash otherwise

[Chaosus]

Read: https://docs.godotengine.org/en/latest/tutorials/performance/thread_safe_apis.html#rendering. Using instantiate() in non-main thread is unrecommended. Use call_deferred(CallDeferred in C#) if you need it (will force them to be created in the main thread).

[raulsntos]

You should be getting an error like:

ERROR: This function in this node (Label) can only be accessed from either the main thread or a thread group.
Use call_deferred() instead.

Since you are using Task.Run, you can easily replace it with Callable.From and then CallDeferred:

Callable.From(() =>
{
    if (!SkipLabel)
    {
        var node = _packedScene.Instantiate<NumberLabel>();
        node.Position = InitialPosition;
        CallDeferred(Node.MethodName.AddChild, node);
    }

    var node2 = _packedScene2.Instantiate<SpriteNode>();
    node2.Position = InitialPosition2;
    CallDeferred(Node.MethodName.AddChild, node2);
}).CallDeferred();

You can also await the next process frame and that should continue in the main thread because of Godot's synchronization context:

await ToSignal(GetTree(), SceneTree.SignalName.ProcessFrame);

[Targma]

Read: https://docs.godotengine.org/en/latest/tutorials/performance/thread_safe_apis.html#rendering. Using instantiate() in non-main thread is unrecommended. Use call_deferred(CallDeferred in C#) if you need it (will force them to be created in the main thread).

I am aware of that documentation suggestion, but with 4.2 came new multi-threading support and based on blog posts https://godotengine.org/article/dev-snapshot-godot-4-1-dev-3/

We continue to improve Godot’s multi-threaded behavior and fix bugs from the recent changes. Such improvements include
 multiple fixes to multi-threaded resource loading ([GH-74405](https://github.com/godotengine/godot/pull/74405), 
[GH-77143](https://github.com/godotengine/godot/pull/77143)), 
multiple fixes to the WorkerThreadPool class ([GH-76945](https://github.com/godotengine/godot/pull/76945), [GH-76999](https://github.com/godotengine/godot/pull/76999)), and 
an early version of the multi-threaded node processing ([GH-75901](https://github.com/godotengine/godot/pull/75901)).
 Thread safety of many engine and editor types has been improved, though this work is still ongoing and will continue into 
Godot 4.2.

this is now officially a feature although in experimental stage As such i have working solution to just use GodotSynchronizationContext and dispatch action. But this takes time from main thread what we want to avoid.

[Targma]

Upon further investigation it appears instancing scenes with control nodes can cause crash.

[J-Ponzo]

I tried to investigate further and I found the following :

• This is not limited to C#. It also crashes with GDScript
• Depending on the type of node, the instanciate() call will crash or not (ex : AudioListener& WorldEnvironmentnever crash but AudioStreamPlayer& NavigationAgent2Dalways end up crashing)
• Some node types only crash if their embedded resource is set (ex : Sprite2D)
• Some node types never crash even if their embedded resource is set (ex : MeshInstance2D)
• Crash does not seem to be correlated with the resource type (Sprite2Dcrashes only if texture is set, MeshInstance2Dworks even if texture is set)

Here is the sample project I investigated with :

multithread-instanciate-crash.zip

It's GDScript only so anyone can open it. That said, GDScript is not my language. Sorry in advance if the code is not very clean

It consists of a simple Main.tscn scene containing a single node with an attached instantiator.gd script. The script simply instanciates the my_packed_scene.tscn scene each frame until a crash occurs.

You can play with the following parameters :

• NbThreads : number of instantiation threads
• InstancesPerFrame: Number of scene instantiations per frame
• UseMutex: encapsulate the Instantiate() call with a mutex

You can add other nodes in my_packed_scene.tscn to check if it crashes or not

my_packed_scene_crash_no_matter_what.tscn is here only to keep tracks of the "Always crashing" nodes

Here is a list of the nodes I tested :

(After 10000 successful frames, I considered it will never crash)

Always crashes :

• AudioStreamPlayer
• NavigationAgent2D
• NavigationAgent3D
• GridMap

Never crashes :

• Label
• AudioListener2D
• ColorRect
• AnimationPlayer
• Timer
• Camera3D
• BoneAttachment

Always crashes if resource is set :

• TextureRect
• MeshInstance3D
• Sprite2D
• StaticBody2D
• CollisionShape2D
• RigidBody3D
• CollisionShape3D
• AnimationTree

Never crashes even if resource is set :

• CustomNode with CustomResource
• MeshInstance2D
• WorldEnvironment

Possibly related issues

• https://github.com/godotengine/godot/issues/68794
• https://github.com/godotengine/godot-cpp/issues/928

Godot version

v4.3.dev6.mono.official [89850d553]

System info

Godot v4.3.dev6.mono - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2080 (NVIDIA; 31.0.15.3623) - Intel(R) Core(TM) i7-4770K CPU @ 3.50GHz (8 Threads)