1
0
mirror of https://github.com/godotengine/godot.git synced 2025-12-03 16:55:53 +00:00

C#: Ensure native handles are freed after switch to .NET Core

Finalizers are longer guaranteed to be called on exit now that
we switched to .NET Core. This results in native instances leaking.

The only solution I can think of so far is to keep a list of all
instances alive to dispose when the AssemblyLoadContext.Unloading
event is raised.
This commit is contained in:
Ignacio Roldán Etcheverry
2021-12-28 23:25:16 +01:00
parent 4d710bf659
commit f88d8902cf
15 changed files with 185 additions and 32 deletions

View File

@@ -13,16 +13,21 @@ namespace Godot.Collections
/// interfacing with the engine. Otherwise prefer .NET collections
/// such as <see cref="System.Array"/> or <see cref="List{T}"/>.
/// </summary>
public sealed class Array : IList, IDisposable
public sealed class Array :
IList,
IDisposable
{
internal godot_array.movable NativeValue;
private WeakReference<IDisposable> _weakReferenceToSelf;
/// <summary>
/// Constructs a new empty <see cref="Array"/>.
/// </summary>
public Array()
{
NativeValue = (godot_array.movable)NativeFuncs.godotsharp_array_new();
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}
/// <summary>
@@ -51,6 +56,8 @@ namespace Godot.Collections
throw new ArgumentNullException(nameof(array));
NativeValue = (godot_array.movable)NativeFuncs.godotsharp_array_new();
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
int length = array.Length;
Resize(length);
@@ -64,6 +71,7 @@ namespace Godot.Collections
NativeValue = (godot_array.movable)(nativeValueToOwn.IsAllocated ?
nativeValueToOwn :
NativeFuncs.godotsharp_array_new());
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}
// Explicit name to make it very clear
@@ -88,6 +96,7 @@ namespace Godot.Collections
{
// Always dispose `NativeValue` even if disposing is true
NativeValue.DangerousSelfRef.Dispose();
DisposablesTracker.UnregisterDisposable(_weakReferenceToSelf);
}
/// <summary>

View File

@@ -33,6 +33,7 @@ namespace Godot.Bridge
public delegate* unmanaged<IntPtr, void> GCHandleBridge_FreeGCHandle;
public delegate* unmanaged<void> DebuggingUtils_InstallTraceListener;
public delegate* unmanaged<void> Dispatcher_InitializeDefaultGodotTaskScheduler;
public delegate* unmanaged<void> DisposablesTracker_OnGodotShuttingDown;
// @formatter:on
public static ManagedCallbacks Create()
@@ -65,6 +66,7 @@ namespace Godot.Bridge
GCHandleBridge_FreeGCHandle = &GCHandleBridge.FreeGCHandle,
DebuggingUtils_InstallTraceListener = &DebuggingUtils.InstallTraceListener,
Dispatcher_InitializeDefaultGodotTaskScheduler = &Dispatcher.InitializeDefaultGodotTaskScheduler,
DisposablesTracker_OnGodotShuttingDown = &DisposablesTracker.OnGodotShuttingDown,
// @formatter:on
};
}

View File

@@ -44,16 +44,19 @@ namespace Godot.Bridge
internal static unsafe IntPtr CreateManagedForGodotObjectBinding(godot_string_name* nativeTypeName,
IntPtr godotObject)
{
// TODO: Optimize with source generators and delegate pointers
try
{
Type nativeType = TypeGetProxyClass(nativeTypeName);
var obj = (Object)FormatterServices.GetUninitializedObject(nativeType);
obj.NativePtr = godotObject;
var ctor = nativeType.GetConstructor(
BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance,
null, Type.EmptyTypes, null);
obj.NativePtr = godotObject;
_ = ctor!.Invoke(obj, null);
return GCHandle.ToIntPtr(GCHandle.Alloc(obj));
@@ -70,14 +73,14 @@ namespace Godot.Bridge
IntPtr godotObject,
godot_variant** args, int argCount)
{
// TODO: Optimize with source generators and delegate pointers
try
{
// Performance is not critical here as this will be replaced with source generators.
Type scriptType = _scriptBridgeMap[scriptPtr];
var obj = (Object)FormatterServices.GetUninitializedObject(scriptType);
obj.NativePtr = godotObject;
var ctor = scriptType
.GetConstructors(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
.Where(c => c.GetParameters().Length == argCount)
@@ -108,7 +111,11 @@ namespace Godot.Bridge
*args[i], parameters[i].ParameterType);
}
ctor.Invoke(obj, invokeParams);
obj.NativePtr = godotObject;
_ = ctor.Invoke(obj, invokeParams);
return true.ToGodotBool();
}
catch (Exception e)

View File

@@ -18,12 +18,15 @@ namespace Godot.Collections
{
internal godot_dictionary.movable NativeValue;
private WeakReference<IDisposable> _weakReferenceToSelf;
/// <summary>
/// Constructs a new empty <see cref="Dictionary"/>.
/// </summary>
public Dictionary()
{
NativeValue = (godot_dictionary.movable)NativeFuncs.godotsharp_dictionary_new();
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}
/// <summary>
@@ -45,6 +48,7 @@ namespace Godot.Collections
NativeValue = (godot_dictionary.movable)(nativeValueToOwn.IsAllocated ?
nativeValueToOwn :
NativeFuncs.godotsharp_dictionary_new());
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}
// Explicit name to make it very clear
@@ -69,6 +73,7 @@ namespace Godot.Collections
{
// Always dispose `NativeValue` even if disposing is true
NativeValue.DangerousSelfRef.Dispose();
DisposablesTracker.UnregisterDisposable(_weakReferenceToSelf);
}
/// <summary>

View File

@@ -0,0 +1,101 @@
using System;
using System.Collections.Concurrent;
using System.Runtime.InteropServices;
using System.Runtime.Loader;
using Godot.NativeInterop;
#nullable enable
namespace Godot
{
internal static class DisposablesTracker
{
static DisposablesTracker()
{
AssemblyLoadContext.Default.Unloading += _ => OnUnloading();
}
[UnmanagedCallersOnly]
internal static void OnGodotShuttingDown()
{
try
{
OnUnloading();
}
catch (Exception e)
{
ExceptionUtils.DebugUnhandledException(e);
}
}
private static void OnUnloading()
{
bool isStdoutVerbose;
try
{
isStdoutVerbose = OS.IsStdoutVerbose();
}
catch (ObjectDisposedException)
{
// OS singleton already disposed. Maybe OnUnloading was called twice.
isStdoutVerbose = false;
}
if (isStdoutVerbose)
GD.Print("Unloading: Disposing tracked instances...");
// Dispose Godot Objects first, and only then dispose other disposables
// like StringName, NodePath, Godot.Collections.Array/Dictionary, etc.
// The Godot Object Dispose() method may need any of the later instances.
foreach (WeakReference<Object> item in GodotObjectInstances.Keys)
{
if (item.TryGetTarget(out Object? self))
self.Dispose();
}
foreach (WeakReference<IDisposable> item in OtherInstances.Keys)
{
if (item.TryGetTarget(out IDisposable? self))
self.Dispose();
}
if (isStdoutVerbose)
GD.Print("Unloading: Finished disposing tracked instances.");
}
// ReSharper disable once RedundantNameQualifier
private static ConcurrentDictionary<WeakReference<Godot.Object>, object?> GodotObjectInstances { get; } =
new();
private static ConcurrentDictionary<WeakReference<IDisposable>, object?> OtherInstances { get; } =
new();
public static WeakReference<Object> RegisterGodotObject(Object godotObject)
{
var weakReferenceToSelf = new WeakReference<Object>(godotObject);
GodotObjectInstances.TryAdd(weakReferenceToSelf, null);
return weakReferenceToSelf;
}
public static WeakReference<IDisposable> RegisterDisposable(IDisposable disposable)
{
var weakReferenceToSelf = new WeakReference<IDisposable>(disposable);
OtherInstances.TryAdd(weakReferenceToSelf, null);
return weakReferenceToSelf;
}
public static void UnregisterGodotObject(WeakReference<Object> weakReference)
{
if (!GodotObjectInstances.TryRemove(weakReference, out _))
throw new ArgumentException("Godot Object not registered", nameof(weakReference));
}
public static void UnregisterDisposable(WeakReference<IDisposable> weakReference)
{
if (!OtherInstances.TryRemove(weakReference, out _))
throw new ArgumentException("Disposable not registered", nameof(weakReference));
}
}
}

View File

@@ -43,6 +43,8 @@ namespace Godot
{
internal godot_node_path.movable NativeValue;
private WeakReference<IDisposable> _weakReferenceToSelf;
~NodePath()
{
Dispose(false);
@@ -61,11 +63,13 @@ namespace Godot
{
// Always dispose `NativeValue` even if disposing is true
NativeValue.DangerousSelfRef.Dispose();
DisposablesTracker.UnregisterDisposable(_weakReferenceToSelf);
}
private NodePath(godot_node_path nativeValueToOwn)
{
NativeValue = (godot_node_path.movable)nativeValueToOwn;
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}
// Explicit name to make it very clear
@@ -111,7 +115,10 @@ namespace Godot
public NodePath(string path)
{
if (!string.IsNullOrEmpty(path))
{
NativeValue = (godot_node_path.movable)NativeFuncs.godotsharp_node_path_new_from_string(path);
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}
}
/// <summary>

View File

@@ -11,7 +11,9 @@ namespace Godot
private Type _cachedType = typeof(Object);
internal IntPtr NativePtr;
internal bool MemoryOwn;
private bool _memoryOwn;
private WeakReference<Object> _weakReferenceToSelf;
/// <summary>
/// Constructs a new <see cref="Object"/>.
@@ -34,6 +36,8 @@ namespace Godot
GetType(), _cachedType);
}
_weakReferenceToSelf = DisposablesTracker.RegisterGodotObject(this);
_InitializeGodotScriptInstanceInternals();
}
@@ -61,7 +65,7 @@ namespace Godot
internal Object(bool memoryOwn)
{
MemoryOwn = memoryOwn;
_memoryOwn = memoryOwn;
}
/// <summary>
@@ -74,7 +78,12 @@ namespace Godot
if (instance == null)
return IntPtr.Zero;
if (instance._disposed)
// We check if NativePtr is null because this may be called by the debugger.
// If the debugger puts a breakpoint in one of the base constructors, before
// NativePtr is assigned, that would result in UB or crashes when calling
// native functions that receive the pointer, which can happen because the
// debugger calls ToString() and tries to get the value of properties.
if (instance._disposed || instance.NativePtr == IntPtr.Zero)
throw new ObjectDisposedException(instance.GetType().FullName);
return instance.NativePtr;
@@ -104,9 +113,8 @@ namespace Godot
if (NativePtr != IntPtr.Zero)
{
if (MemoryOwn)
if (_memoryOwn)
{
MemoryOwn = false;
NativeFuncs.godotsharp_internal_refcounted_disposed(NativePtr, (!disposing).ToGodotBool());
}
else
@@ -117,6 +125,8 @@ namespace Godot
NativePtr = IntPtr.Zero;
}
DisposablesTracker.UnregisterGodotObject(_weakReferenceToSelf);
_disposed = true;
}

View File

@@ -14,6 +14,8 @@ namespace Godot
{
internal godot_string_name.movable NativeValue;
private WeakReference<IDisposable> _weakReferenceToSelf;
~StringName()
{
Dispose(false);
@@ -32,11 +34,13 @@ namespace Godot
{
// Always dispose `NativeValue` even if disposing is true
NativeValue.DangerousSelfRef.Dispose();
DisposablesTracker.UnregisterDisposable(_weakReferenceToSelf);
}
private StringName(godot_string_name nativeValueToOwn)
{
NativeValue = (godot_string_name.movable)nativeValueToOwn;
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}
// Explicit name to make it very clear
@@ -57,7 +61,10 @@ namespace Godot
public StringName(string name)
{
if (!string.IsNullOrEmpty(name))
{
NativeValue = (godot_string_name.movable)NativeFuncs.godotsharp_string_name_new_from_string(name);
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}
}
/// <summary>
@@ -138,5 +145,15 @@ namespace Godot
{
return NativeValue.DangerousSelfRef == other;
}
public override bool Equals(object obj)
{
return ReferenceEquals(this, obj) || (obj is StringName other && Equals(other));
}
public override int GetHashCode()
{
return NativeValue.GetHashCode();
}
}
}

View File

@@ -60,6 +60,7 @@
<Compile Include="Core\GodotTaskScheduler.cs" />
<Compile Include="Core\GodotTraceListener.cs" />
<Compile Include="Core\GodotUnhandledExceptionEvent.cs" />
<Compile Include="Core\DisposablesTracker.cs" />
<Compile Include="Core\Interfaces\IAwaitable.cs" />
<Compile Include="Core\Interfaces\IAwaiter.cs" />
<Compile Include="Core\Interfaces\ISerializationListener.cs" />