Prevent Unexpected GC of Valid GodotObject

[scgm0]

GC may accidentally collect a GodotObject instance since DisposablesTracker only holds a WeakReference to it.

While the DisposablesTracker has applied a similar mechanism in the GodotSharp, a direct reference to the GodotObject instance is retained in CustomGCHandle to keep the GodotObject alive.

godot-dotnet currently does not implement such CustomGCHandle, so I added a direct reference to the GodotObject instance in the DisposablesTracker instead. Still, the RefCounted instance continues to hold a weak reference, just like what GodotSharp does.

[takethebait]

Just tested these changes and unfortunately they do not fix #20 for me. I still get the same unref errors:

[scgm0]

Just tested these changes and unfortunately they do not fix #20 for me. I still get the same unref errors:

It seems they are two different issues, I just don't know why they always come up at the same time in my case

[scgm0]

@raulsntos I found that Callable also has unexpected GC problems, which makes the signal linking method of += completely unavailable. Even if you use Connect to manually link the signal, you need to keep a separate reference for the instance created by Callable.From. Only in this way can we ensure that the signal link method can always be executed normally. Should we not use weak references for Callable?

[raulsntos]

I've noticed the same issues and I made the following changes locally. I was considering pushing these changes but I was on the fence about it.

diff --git a/src/Godot.Bindings/Bridge/Callables/CustomCallable.cs b/src/Godot.Bindings/Bridge/Callables/CustomCallable.cs
index 6032ff5..09f2809 100644
--- a/src/Godot.Bindings/Bridge/Callables/CustomCallable.cs
+++ b/src/Godot.Bindings/Bridge/Callables/CustomCallable.cs
@@ -25,7 +25,7 @@ public abstract class CustomCallable
 
     internal unsafe NativeGodotCallable ConstructCallable()
     {
-        var gcHandle = GCHandle.Alloc(this, GCHandleType.Weak);
+        var gcHandle = GCHandle.Alloc(this, GCHandleType.Normal);
 
         var info = new GDExtensionCallableCustomInfo2()
         {
diff --git a/src/Godot.Bindings/Bridge/ClassDB/ClassDBRegistrationContext.cs b/src/Godot.Bindings/Bridge/ClassDB/ClassDBRegistrationContext.cs
index 1ee4309..e374532 100644
--- a/src/Godot.Bindings/Bridge/ClassDB/ClassDBRegistrationContext.cs
+++ b/src/Godot.Bindings/Bridge/ClassDB/ClassDBRegistrationContext.cs
@@ -13,7 +13,7 @@ public partial class ClassDBRegistrationContext
 
     internal ClassDBRegistrationContext(StringName className)
     {
-        GCHandle = GCHandle.Alloc(this, GCHandleType.Weak);
+        GCHandle = GCHandle.Alloc(this, GCHandleType.Normal);
         ClassName = className;
     }
 }
diff --git a/src/Godot.Bindings/Core/GodotObject.cs b/src/Godot.Bindings/Core/GodotObject.cs
index d4393f1..9b480f2 100644
--- a/src/Godot.Bindings/Core/GodotObject.cs
+++ b/src/Godot.Bindings/Core/GodotObject.cs
@@ -26,7 +26,7 @@ partial class GodotObject : IDisposable
     /// <param name="nativePtr">The pointer to the native object in the engine's side.</param>
     internal protected GodotObject(nint nativePtr)
     {
-        _gcHandle = GCHandle.Alloc(this, GCHandleType.Weak);
+        _gcHandle = GCHandle.Alloc(this, GCHandleType.Normal);
 
         NativePtr = nativePtr;
 

[scgm0]

I've noticed the same issues and I made the following changes locally. I was considering pushing these changes but I was on the fence about it.

diff --git a/src/Godot.Bindings/Bridge/Callables/CustomCallable.cs b/src/Godot.Bindings/Bridge/Callables/CustomCallable.cs
index 6032ff5..09f2809 100644
--- a/src/Godot.Bindings/Bridge/Callables/CustomCallable.cs
+++ b/src/Godot.Bindings/Bridge/Callables/CustomCallable.cs
@@ -25,7 +25,7 @@ public abstract class CustomCallable
 
     internal unsafe NativeGodotCallable ConstructCallable()
     {
-        var gcHandle = GCHandle.Alloc(this, GCHandleType.Weak);
+        var gcHandle = GCHandle.Alloc(this, GCHandleType.Normal);
 
         var info = new GDExtensionCallableCustomInfo2()
         {
diff --git a/src/Godot.Bindings/Bridge/ClassDB/ClassDBRegistrationContext.cs b/src/Godot.Bindings/Bridge/ClassDB/ClassDBRegistrationContext.cs
index 1ee4309..e374532 100644
--- a/src/Godot.Bindings/Bridge/ClassDB/ClassDBRegistrationContext.cs
+++ b/src/Godot.Bindings/Bridge/ClassDB/ClassDBRegistrationContext.cs
@@ -13,7 +13,7 @@ public partial class ClassDBRegistrationContext
 
     internal ClassDBRegistrationContext(StringName className)
     {
-        GCHandle = GCHandle.Alloc(this, GCHandleType.Weak);
+        GCHandle = GCHandle.Alloc(this, GCHandleType.Normal);
         ClassName = className;
     }
 }
diff --git a/src/Godot.Bindings/Core/GodotObject.cs b/src/Godot.Bindings/Core/GodotObject.cs
index d4393f1..9b480f2 100644
--- a/src/Godot.Bindings/Core/GodotObject.cs
+++ b/src/Godot.Bindings/Core/GodotObject.cs
@@ -26,7 +26,7 @@ partial class GodotObject : IDisposable
     /// <param name="nativePtr">The pointer to the native object in the engine's side.</param>
     internal protected GodotObject(nint nativePtr)
     {
-        _gcHandle = GCHandle.Alloc(this, GCHandleType.Weak);
+        _gcHandle = GCHandle.Alloc(this, GCHandleType.Normal);
 
         NativePtr = nativePtr;
 

Looks good, I'll try it

[Delsin-Yu]

Changing the HandleType from Weak to Normal means we must ensuring the GDExtension system and the interop layer never forget to free the handle after its scope; otherwise, native memory leaks would infect the managed side.

But I think we can do experiments as this project is still a WIP?