From 6cb9fe05a20f949b83feb994bda639ee3ff3cea4 Mon Sep 17 00:00:00 2001 From: badcel <1218031+badcel@users.noreply.github.com> Date: Sun, 29 Sep 2024 16:57:22 +0200 Subject: [PATCH] Object: Do not toggle reference after handle was already freed It can happen that calling "RemoveToggleRef" leads to a delayed invocation of it's toggle notify callback. If "RemoveToggleRef" is called the garbage collector is free to free the ToggleRef in which case the managed "ToggleNotify" callback is freed, too. This change uses an "UnmanagedCallersOnly" function which is out of scope of the GC and can always be called. In case it is called after the ToggleRef was freed it just does nothing. Fixes: #1125 --- .../Internal/ObjectMapper.ToggleRef.cs | 23 +--------- .../ObjectMapper.ToggleRegistration.cs | 32 +++++++++++++ src/Libs/GObject-2.0/Internal/ObjectMapper.cs | 45 +++++++++++++++---- 3 files changed, 70 insertions(+), 30 deletions(-) create mode 100644 src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRegistration.cs diff --git a/src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRef.cs b/src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRef.cs index efe9d2c51..cd54ed862 100644 --- a/src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRef.cs +++ b/src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRef.cs @@ -5,10 +5,9 @@ namespace GObject.Internal; public partial class ObjectMapper { - private class ToggleRef : IDisposable + private class ToggleRef { private object _reference; - private readonly ToggleNotify _callback; private readonly IntPtr _handle; public object? Object @@ -40,17 +39,9 @@ public object? Object public ToggleRef(IntPtr handle, object obj, bool ownedRef) { _reference = obj; - _callback = ToggleReference; _handle = handle; OwnReference(ownedRef); - RegisterToggleRef(); - } - - private void RegisterToggleRef() - { - Internal.Object.AddToggleRef(_handle, _callback, IntPtr.Zero); - Internal.Object.Unref(_handle); } private void OwnReference(bool ownedRef) @@ -72,7 +63,7 @@ private void OwnReference(bool ownedRef) } } - private void ToggleReference(IntPtr data, IntPtr @object, bool isLastRef) + internal void ToggleReference(bool isLastRef) { if (!isLastRef && _reference is WeakReference weakRef) { @@ -86,15 +77,5 @@ private void ToggleReference(IntPtr data, IntPtr @object, bool isLastRef) _reference = new WeakReference(_reference); } } - - public void Dispose() - { - var sourceFunc = new GLib.Internal.SourceFuncAsyncHandler(() => - { - Internal.Object.RemoveToggleRef(_handle, _callback, IntPtr.Zero); - return false; - }); - GLib.Internal.MainContext.Invoke(GLib.Internal.MainContextUnownedHandle.NullHandle, sourceFunc.NativeCallback, IntPtr.Zero); - } } } diff --git a/src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRegistration.cs b/src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRegistration.cs new file mode 100644 index 000000000..0cc784e44 --- /dev/null +++ b/src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRegistration.cs @@ -0,0 +1,32 @@ +using System; +using System.Runtime.InteropServices; + +namespace GObject.Internal; + +public partial class ObjectMapper +{ + private static unsafe class ToggleRegistration + { + internal static void AddToggleRef(IntPtr handle) + { + AddToggleRef(handle, &ToggleNotify, IntPtr.Zero); + Internal.Object.Unref(handle); + } + + internal static void RemoveToggleRef(IntPtr handle) + { + var sourceFunc = new GLib.Internal.SourceFuncAsyncHandler(() => + { + RemoveToggleRef(handle, &ToggleNotify, IntPtr.Zero); + return false; + }); + GLib.Internal.MainContext.Invoke(GLib.Internal.MainContextUnownedHandle.NullHandle, sourceFunc.NativeCallback, IntPtr.Zero); + } + + [DllImport(ImportResolver.Library, EntryPoint = "g_object_add_toggle_ref")] + private static extern void AddToggleRef(IntPtr @object, delegate* unmanaged toggleNotify, IntPtr data); + + [DllImport(ImportResolver.Library, EntryPoint = "g_object_remove_toggle_ref")] + private static extern void RemoveToggleRef(IntPtr @object, delegate* unmanaged toggleNotify, IntPtr data); + } +} diff --git a/src/Libs/GObject-2.0/Internal/ObjectMapper.cs b/src/Libs/GObject-2.0/Internal/ObjectMapper.cs index e50a8775f..80bc5c5f4 100644 --- a/src/Libs/GObject-2.0/Internal/ObjectMapper.cs +++ b/src/Libs/GObject-2.0/Internal/ObjectMapper.cs @@ -2,24 +2,38 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices; using GLib; namespace GObject.Internal; public static partial class ObjectMapper { + private static readonly object Lock = new(); private static readonly Dictionary WrapperObjects = new(); - public static int ObjectCount => WrapperObjects.Count; + public static int ObjectCount + { + get + { + lock (Lock) + { + return WrapperObjects.Count; + } + } + } public static bool TryGetObject(IntPtr handle, [NotNullWhen(true)] out T? obj) where T : class, IHandle { - if (WrapperObjects.TryGetValue(handle, out ToggleRef? weakRef)) + lock (Lock) { - if (weakRef.Object is not null) + if (WrapperObjects.TryGetValue(handle, out ToggleRef? weakRef)) { - obj = (T) weakRef.Object; - return true; + if (weakRef.Object is not null) + { + obj = (T) weakRef.Object; + return true; + } } } @@ -29,9 +43,10 @@ public static bool TryGetObject(IntPtr handle, [NotNullWhen(true)] out T? obj public static void Map(IntPtr handle, object obj, bool ownedRef) { - lock (WrapperObjects) + lock (Lock) { WrapperObjects[handle] = new ToggleRef(handle, obj, ownedRef); + ToggleRegistration.AddToggleRef(handle); } Debug.WriteLine($"Handle {handle}: Mapped object of type '{obj.GetType()}' as owned ref '{ownedRef}'."); @@ -39,12 +54,24 @@ public static void Map(IntPtr handle, object obj, bool ownedRef) public static void Unmap(IntPtr handle) { - lock (WrapperObjects) + lock (Lock) { - if (WrapperObjects.Remove(handle, out var toggleRef)) - toggleRef.Dispose(); + if (WrapperObjects.Remove(handle)) + ToggleRegistration.RemoveToggleRef(handle); } Debug.WriteLine($"Handle {handle}: Unmapped object."); } + + [UnmanagedCallersOnly] + private static void ToggleNotify(IntPtr data, IntPtr @object, int isLastRef) + { + lock (Lock) + { + if (WrapperObjects.TryGetValue(@object, out var toggleRef)) + toggleRef.ToggleReference(isLastRef != 0); + else + Debug.WriteLine($"Handle {@object}: Could not toggle to {isLastRef} as there is no toggle reference."); + } + } }