C# Rendering Device might dispose resources at wrong thread when using Compositor Effect

[hippogyz]

Tested versions

• Reproducible in v4.4.stable.mono
• Not producible in v4.3.stable.mono

System information

Godot v4.4.stable.mono - Windows 11 (build 22631) - Multi-window, 2 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4070 (NVIDIA; 32.0.15.6094) - AMD Ryzen 9 7900X 12-Core Processor (24 threads)

Issue description

I'm making a rendering effect just like Octopath Traveller's in-house scene, which rendering in-house scene and out-house scene simultaneously, such as

I use two SubViewport to render the inside and outside scene, and use a Compositor Effect to capture the depth texture of the inside scene for final mixing.

However, after I do a main scene transition (unloads all nodes including house and outside scene), and when the next time I do transition back to this scene, Godot Editor reports Bugs as follows (which repeats around 30+ times for one scene transition)

E 0:00:22:180   GodotObject.base.cs:137 @ void Godot.GodotObject.Dispose(bool): This function (free) can only be called from the render thread. 
  <C++ Error>   Condition "render_thread_id != Thread::get_caller_id()" is true.
  <C++ Source>  servers/rendering/rendering_device.cpp:5995 @ free()
  <Stack Trace> GodotObject.base.cs:137 @ void Godot.GodotObject.Dispose(bool)
                GodotObject.base.cs:100 @ void Godot.GodotObject.Finalize()

I'm not sure how to reproduce it in a MRP, because it seems this bug report triggers from C# GodotObject.Dispose(). I've tried use GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced); GC.WaitForPendingFinalizers(); manually in MRP, but it cannot reproduce the bug report.

I suspect this bug report is corresponding to the Compositor Effect. Because the bug report only appears after I have entered the house and created the Compositor Effect.

Steps to reproduce

1. make a scene (which calls SCENE_A), add Compositor Effect to a camera.
2. do scene transition to another scene.
3. do scene transition again, and editor will report bugs.

Minimal reproduction project (MRP)

test_free_rid.zip

In this MRP, press 'F' to turn on/off Depth Capture, press 'Esc' to switch scene and trigger GC. Debugger will report Errors if Depth Capture has been turned on once.

[hippogyz]

Update MRP.

[clayjohn]

@hippogyz my guess is that you are leaking a node that is getting cleaned up by the garbage collector. As a result, the free function will get called from whatever thread the garbage collector is run on, which likely isn't the rendering thread.

You need to manually free the rendering resources in order to avoid this

[hippogyz]

@hippogyz my guess is that you are leaking a node that is getting cleaned up by the garbage collector. As a result, the free function will get called from whatever thread the garbage collector is run on, which likely isn't the rendering thread.

You need to manually free the rendering resources in order to avoid this

Well...I tried to comment some codes in the MRP, and I found this line caused resources leaking:

    public override void _RenderCallback(int effectCallbackType, RenderData renderData)
    {
        var renderSceneBuffers = renderData.GetRenderSceneBuffers() as RenderSceneBuffersRD;
        ... // comment rest of codes in _RenderCallback still triggers the Error

It seems RenderData.GetRenderSceneBuffers() leaks any resources internally?

EDIT: I tried other members in RenderData, only the RenderSceneBuffers will triggers the error. Maybe the problem is that the RenderSceneBuffers is a RefCounted?

[hippogyz]

A workaround:

1. rewrite your CompositorEffect with gdscript.
2. manually instantiate it in C#.
3. when you no longer need it, dereference the GodotObject instance.
4. now Editor won't throw errors when scene switching.

Example: test_free_rid.zip
Enable/disable the UseCSharp flag in LoadEffect.cs to see the difference between C# case and gdscript case.

[JeeeJeee]

Is there a proper solution to this other than a workaround? I have run into the same thing and while it "seemingly" doesn't cause any issues it is incredibly annoying to be spammed with errors everytime I open the editor.

[raulsntos]

Looking at the provided stack trace, it looks like the finalizer is being executed. As @clayjohn said, finalizers are executed on a different thread from the rendering thread, so if these resources must be freed on the rendering thread you'll need to properly dispose them to avoid this.

The finalizer is meant to be a fallback, you shouldn't rely on it to clean up resources for you. Instead, disposable objects should always be properly disposed of, by calling the Dispose method or with the using statement ¹.

@hippogyz, as you suspected, it looks like you are not disposing the RenderSceneBuffersRD instance. Consider adding a using statement, like this:

using var renderSceneBuffers = renderData.GetRenderSceneBuffers() as RenderSceneBuffersRD;

This should ensure the resource is properly disposed when you are done using it, instead of leaving it to the garbage collector.

Footnotes

1. https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/using-objects ↩

[hippogyz]

Looking at the provided stack trace, it looks like the finalizer is being executed. As @clayjohn said, finalizers are executed on a different thread from the rendering thread, so if these resources must be freed on the rendering thread you'll need to properly dispose them to avoid this.

The finalizer is meant to be a fallback, you shouldn't rely on it to clean up resources for you. Instead, disposable objects should always be properly disposed of, by calling the Dispose method or with the using statement 1.

@hippogyz, as you suspected, it looks like you are not disposing the RenderSceneBuffersRD instance. Consider adding a using statement, like this:

using var renderSceneBuffers = renderData.GetRenderSceneBuffers() as RenderSceneBuffersRD;
This should ensure the resource is properly disposed when you are done using it, instead of leaving it to the garbage collector.

Footnotes

1. https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/using-objects ↩

It works for me. Thanks!

But there is one thing I don't understand:
In this case, the inherit order is RefCounted -> RenderSceneBuffers -> RenderSceneBuffersRD. But in my understanding, C# as won't create a new instance for this object. So using statement will actually dispose the RenderSceneBuffers which is a member of input parameter RenderData in the _RenderCallback() function. Does it make the object's lifetime unmatched with its scope?

(EDIT: I read the GodotObject.cs and runtime_interop.cpp. It seems dispose C# object only reduce the ref-count but won't free the object instantaneous? )

Anyway, maybe using using statement for C# CompositorEffect should be noted in document.

[raulsntos]

When you get a reference to a RefCounted object in C#, it increments the reference count; disposing it decrements the reference count. When the reference count reaches 0, the resource is freed.

You are correct that the as operator doesn't create new instances and therefore it doesn't affect the reference count. It's just a cast that returns null if the cast fails, instead of throwing an exception like a regular cast would.

You are also correct that you usually shouldn't dispose input parameters, since you don't normally own them. This depends on the specific scenario, but it makes sense to assume that if you didn't create the instance you receive as an input parameter, you probably aren't responsible for disposing it.

However, in this particular case, the instance we're referring to is not the renderData input parameter itself. The method GetRenderSceneBuffers() returns a new RefCounted instance. This means GetRenderSceneBuffers() is incrementing the reference count, and you are now responsible for disposing it. The fact that this method is called on an input parameter is not relevant to the ownership of the returned object.

Anyway, maybe using using statement for C# CompositorEffect should be noted in document.

The using statement is relevant for all C# types that implement the IDisposable interface, not just CompositorEffect, and it's more of a .NET concept than a Godot one. That said, I think it makes sense to add a note about it (and link to Microsoft's documentation) for the types where it's specially relevant such as those that need to be disposed in the rendering thread.

[hippogyz]

Thanks for explaining. This issue should be closed now.

I've added a comment to the Compositor document.

And for someone who meets the same question: use using statement for RenderSceneBuffersRD, i.e.

using var renderSceneBuffers = renderData.GetRenderSceneBuffers() as RenderSceneBuffersRD;