Skip to content

Commit

Permalink
Object: Do not toggle reference after handle was already freed
Browse files Browse the repository at this point in the history
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
  • Loading branch information
badcel committed Oct 3, 2024
1 parent 4edc18a commit 6cb9fe0
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 30 deletions.
23 changes: 2 additions & 21 deletions src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRef.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
{
Expand All @@ -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);
}
}
}
32 changes: 32 additions & 0 deletions src/Libs/GObject-2.0/Internal/ObjectMapper.ToggleRegistration.cs
Original file line number Diff line number Diff line change
@@ -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<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);
}
}
45 changes: 36 additions & 9 deletions src/Libs/GObject-2.0/Internal/ObjectMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand All @@ -29,22 +43,35 @@ 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);
ToggleRegistration.AddToggleRef(handle);
}

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();
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.");
}
}
}

0 comments on commit 6cb9fe0

Please sign in to comment.