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

src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRef.cs

@@ -1,14 +1,14 @@
 using System;
 using System.Diagnostics;
+using System.Runtime.InteropServices;
 
 namespace GObject.Internal;
 
 public partial class ObjectMapper
 {
-    private class ToggleRef : IDisposable
+    private unsafe class ToggleRef : IDisposable
     {
         private object _reference;
-        private readonly ToggleNotify _callback;
         private readonly IntPtr _handle;
 
         public object? Object
@@ -40,16 +40,14 @@ public object? Object
         public ToggleRef(IntPtr handle, object obj, bool ownedRef)
         {
             _reference = obj;
-            _callback = ToggleReference;
             _handle = handle;
 
             OwnReference(ownedRef);
-            RegisterToggleRef();
         }
 
-        private void RegisterToggleRef()
+        internal void RegisterToggleRef()
         {
-            Internal.Object.AddToggleRef(_handle, _callback, IntPtr.Zero);
+            AddToggleRef(_handle, &ToggleNotify, IntPtr.Zero);
             Internal.Object.Unref(_handle);
         }
 
@@ -72,7 +70,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)
             {
@@ -91,10 +89,16 @@ public void Dispose()
         {
             var sourceFunc = new GLib.Internal.SourceFuncAsyncHandler(() =>
             {
-                Internal.Object.RemoveToggleRef(_handle, _callback, IntPtr.Zero);
+                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<IntPtr, IntPtr, int, void> toggleNotify, IntPtr data);
+
+        [DllImport(ImportResolver.Library, EntryPoint = "g_object_remove_toggle_ref")]
+        private static extern void RemoveToggleRef(IntPtr @object, delegate* unmanaged<IntPtr, IntPtr, int, void> toggleNotify, IntPtr data);
     }
 }

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<IntPtr, ToggleRef> WrapperObjects = new();
 
-    public static int ObjectCount => WrapperObjects.Count;
+    public static int ObjectCount
+    {
+        get
+        {
+            lock (Lock)
+            {
+                return WrapperObjects.Count;
+            }
+        }
+    }
 
     public static bool TryGetObject<T>(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,22 +43,36 @@ public static bool TryGetObject<T>(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);
+            var toggleRef = new ToggleRef(handle, obj, ownedRef);
+            WrapperObjects[handle] = toggleRef;
+            toggleRef.RegisterToggleRef();
         }
 
         Debug.WriteLine($"Handle {handle}: Mapped object of type '{obj.GetType()}' as owned ref '{ownedRef}'.");
     }
 
     public static void Unmap(IntPtr handle)
     {
-        lock (WrapperObjects)
+        lock (Lock)
         {
             if (WrapperObjects.Remove(handle, out var toggleRef))
                 toggleRef.Dispose();
         }
 
         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.");
+        }
+    }
 }