diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index 80e127bbc2c..52b0e82c6e7 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -1581,6 +1581,7 @@ void CSharpLanguage::tie_managed_to_unmanaged_with_pre_setup(GCHandleIntPtr p_gc CSharpInstance *instance = CAST_CSHARP_INSTANCE(p_unmanaged->get_script_instance()); if (!instance) { + // Native bindings don't need post-setup return; } diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp index 89265d8f2b7..fdc50e22f34 100644 --- a/modules/mono/editor/bindings_generator.cpp +++ b/modules/mono/editor/bindings_generator.cpp @@ -75,6 +75,10 @@ StringBuilder &operator<<(StringBuilder &r_sb, const char *p_cstring) { #define CLOSE_BLOCK_L3 INDENT3 CLOSE_BLOCK #define CLOSE_BLOCK_L4 INDENT4 CLOSE_BLOCK +#define BINDINGS_GLOBAL_SCOPE_CLASS "GD" +#define BINDINGS_PTR_FIELD "NativePtr" +#define BINDINGS_NATIVE_NAME_FIELD "NativeName" + #define CS_PARAM_MEMORYOWN "memoryOwn" #define CS_PARAM_METHODBIND "method" #define CS_PARAM_INSTANCE "ptr" diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs index 9fa221a0cca..cd6655b857c 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs @@ -13,16 +13,21 @@ namespace Godot.Collections /// interfacing with the engine. Otherwise prefer .NET collections /// such as or . /// - public sealed class Array : IList, IDisposable + public sealed class Array : + IList, + IDisposable { internal godot_array.movable NativeValue; + private WeakReference _weakReferenceToSelf; + /// /// Constructs a new empty . /// public Array() { NativeValue = (godot_array.movable)NativeFuncs.godotsharp_array_new(); + _weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this); } /// @@ -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); } /// diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ManagedCallbacks.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ManagedCallbacks.cs index 1d19376cdda..bd939ef27d0 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ManagedCallbacks.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ManagedCallbacks.cs @@ -33,6 +33,7 @@ namespace Godot.Bridge public delegate* unmanaged GCHandleBridge_FreeGCHandle; public delegate* unmanaged DebuggingUtils_InstallTraceListener; public delegate* unmanaged Dispatcher_InitializeDefaultGodotTaskScheduler; + public delegate* unmanaged 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 }; } diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs index 689d6cddbb5..e87b7f4d4bf 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs @@ -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) diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs index 89fc2210b81..4a99872e7b4 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs @@ -18,12 +18,15 @@ namespace Godot.Collections { internal godot_dictionary.movable NativeValue; + private WeakReference _weakReferenceToSelf; + /// /// Constructs a new empty . /// public Dictionary() { NativeValue = (godot_dictionary.movable)NativeFuncs.godotsharp_dictionary_new(); + _weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this); } /// @@ -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); } /// diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/DisposablesTracker.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/DisposablesTracker.cs new file mode 100644 index 00000000000..bf3b3b083a5 --- /dev/null +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/DisposablesTracker.cs @@ -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 item in GodotObjectInstances.Keys) + { + if (item.TryGetTarget(out Object? self)) + self.Dispose(); + } + + foreach (WeakReference 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, object?> GodotObjectInstances { get; } = + new(); + + private static ConcurrentDictionary, object?> OtherInstances { get; } = + new(); + + public static WeakReference RegisterGodotObject(Object godotObject) + { + var weakReferenceToSelf = new WeakReference(godotObject); + GodotObjectInstances.TryAdd(weakReferenceToSelf, null); + return weakReferenceToSelf; + } + + public static WeakReference RegisterDisposable(IDisposable disposable) + { + var weakReferenceToSelf = new WeakReference(disposable); + OtherInstances.TryAdd(weakReferenceToSelf, null); + return weakReferenceToSelf; + } + + public static void UnregisterGodotObject(WeakReference weakReference) + { + if (!GodotObjectInstances.TryRemove(weakReference, out _)) + throw new ArgumentException("Godot Object not registered", nameof(weakReference)); + } + + public static void UnregisterDisposable(WeakReference weakReference) + { + if (!OtherInstances.TryRemove(weakReference, out _)) + throw new ArgumentException("Disposable not registered", nameof(weakReference)); + } + } +} diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/NodePath.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/NodePath.cs index d7b736fbcf0..6edc19c4d62 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/NodePath.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/NodePath.cs @@ -43,6 +43,8 @@ namespace Godot { internal godot_node_path.movable NativeValue; + private WeakReference _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); + } } /// diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Object.base.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Object.base.cs index dbffd1d5d1f..a3a4c2599e5 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Object.base.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Object.base.cs @@ -11,7 +11,9 @@ namespace Godot private Type _cachedType = typeof(Object); internal IntPtr NativePtr; - internal bool MemoryOwn; + private bool _memoryOwn; + + private WeakReference _weakReferenceToSelf; /// /// Constructs a new . @@ -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; } /// @@ -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; } diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/StringName.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/StringName.cs index 3a415d3deb3..b993a1b3e9b 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/StringName.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/StringName.cs @@ -14,6 +14,8 @@ namespace Godot { internal godot_string_name.movable NativeValue; + private WeakReference _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); + } } /// @@ -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(); + } } } diff --git a/modules/mono/glue/GodotSharp/GodotSharp/GodotSharp.csproj b/modules/mono/glue/GodotSharp/GodotSharp/GodotSharp.csproj index d5bbbfb7ca7..f1a397f8fa2 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/GodotSharp.csproj +++ b/modules/mono/glue/GodotSharp/GodotSharp/GodotSharp.csproj @@ -60,6 +60,7 @@ + diff --git a/modules/mono/godotsharp_defs.h b/modules/mono/godotsharp_defs.h index 872c875cbe8..a81a52e4b87 100644 --- a/modules/mono/godotsharp_defs.h +++ b/modules/mono/godotsharp_defs.h @@ -33,15 +33,10 @@ #define BINDINGS_NAMESPACE "Godot" #define BINDINGS_NAMESPACE_COLLECTIONS BINDINGS_NAMESPACE ".Collections" -#define BINDINGS_NAMESPACE_BRIDGE BINDINGS_NAMESPACE ".Bridge" -#define BINDINGS_GLOBAL_SCOPE_CLASS "GD" -#define BINDINGS_PTR_FIELD "NativePtr" -#define BINDINGS_NATIVE_NAME_FIELD "NativeName" #define API_SOLUTION_NAME "GodotSharp" #define CORE_API_ASSEMBLY_NAME "GodotSharp" #define EDITOR_API_ASSEMBLY_NAME "GodotSharpEditor" #define TOOLS_ASM_NAME "GodotTools" -#define TOOLS_PROJECT_EDITOR_ASM_NAME "GodotTools.ProjectEditor" #define BINDINGS_CLASS_NATIVECALLS "NativeCalls" #define BINDINGS_CLASS_NATIVECALLS_EDITOR "EditorNativeCalls" diff --git a/modules/mono/mono_gd/gd_mono.cpp b/modules/mono/mono_gd/gd_mono.cpp index dcda799f32d..7a3fd1af107 100644 --- a/modules/mono/mono_gd/gd_mono.cpp +++ b/modules/mono/mono_gd/gd_mono.cpp @@ -471,26 +471,17 @@ GDMono::GDMono() { } GDMono::~GDMono() { + finalizing_scripts_domain = true; + if (is_runtime_initialized()) { -#warning "TODO assembly unloading for cleanup of disposables (including managed RefCounteds)" -#if 0 - if (scripts_domain) { - Error err = _unload_scripts_domain(); - if (err != OK) { - ERR_PRINT("Mono: Failed to unload scripts domain."); - } + if (GDMonoCache::godot_api_cache_updated) { + GDMonoCache::managed_callbacks.DisposablesTracker_OnGodotShuttingDown(); } - - print_verbose("Mono: Runtime cleanup..."); - - mono_jit_cleanup(root_domain); - - print_verbose("Mono: Finalized"); -#endif - - runtime_initialized = false; } + finalizing_scripts_domain = false; + runtime_initialized = false; + #if defined(ANDROID_ENABLED) gdmono::android::support::cleanup(); #endif diff --git a/modules/mono/mono_gd/gd_mono_cache.cpp b/modules/mono/mono_gd/gd_mono_cache.cpp index 17addfb49d7..e8b25cb1197 100644 --- a/modules/mono/mono_gd/gd_mono_cache.cpp +++ b/modules/mono/mono_gd/gd_mono_cache.cpp @@ -68,6 +68,7 @@ void update_godot_api_cache(const ManagedCallbacks &p_managed_callbacks) { CHECK_CALLBACK_NOT_NULL(GCHandleBridge, FreeGCHandle); CHECK_CALLBACK_NOT_NULL(DebuggingUtils, InstallTraceListener); CHECK_CALLBACK_NOT_NULL(Dispatcher, InitializeDefaultGodotTaskScheduler); + CHECK_CALLBACK_NOT_NULL(DisposablesTracker, OnGodotShuttingDown); managed_callbacks = p_managed_callbacks; diff --git a/modules/mono/mono_gd/gd_mono_cache.h b/modules/mono/mono_gd/gd_mono_cache.h index 56bf4cef949..17c8c9fa512 100644 --- a/modules/mono/mono_gd/gd_mono_cache.h +++ b/modules/mono/mono_gd/gd_mono_cache.h @@ -78,6 +78,7 @@ struct ManagedCallbacks { using FuncGCHandleBridge_FreeGCHandle = void(GD_CLR_STDCALL *)(GCHandleIntPtr); using FuncDebuggingUtils_InstallTraceListener = void(GD_CLR_STDCALL *)(); using FuncDispatcher_InitializeDefaultGodotTaskScheduler = void(GD_CLR_STDCALL *)(); + using FuncDisposablesTracker_OnGodotShuttingDown = void(GD_CLR_STDCALL *)(); FuncSignalAwaiter_SignalCallback SignalAwaiter_SignalCallback; FuncDelegateUtils_InvokeWithVariantArgs DelegateUtils_InvokeWithVariantArgs; @@ -104,6 +105,7 @@ struct ManagedCallbacks { FuncGCHandleBridge_FreeGCHandle GCHandleBridge_FreeGCHandle; FuncDebuggingUtils_InstallTraceListener DebuggingUtils_InstallTraceListener; FuncDispatcher_InitializeDefaultGodotTaskScheduler Dispatcher_InitializeDefaultGodotTaskScheduler; + FuncDisposablesTracker_OnGodotShuttingDown DisposablesTracker_OnGodotShuttingDown; }; extern ManagedCallbacks managed_callbacks;