Skip to content

Commit

Permalink
Ensure recorded delegates are not released too early (#141)
Browse files Browse the repository at this point in the history
The operation cache of libvips could reuse the image after
subsequent use, so we need to free the recorded delegates
until we are sure that the reference count reaches zero.
  • Loading branch information
kleisauke committed Sep 23, 2021
1 parent 76b996e commit 8682c66
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 58 deletions.
2 changes: 1 addition & 1 deletion samples/NetVips.Samples/Samples/GenerateImageClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ string ToCref(string name) =>
if (firstArgType == GValue.SourceType)
{
result.AppendLine()
.AppendLine($"{indent} image.OnUnref += () => source.Dispose();")
.AppendLine($"{indent} image.OnPostClose += () => source.Dispose();")
.AppendLine()
.AppendLine($"{indent} return image;");
}
Expand Down
38 changes: 38 additions & 0 deletions samples/NetVips.Samples/Samples/NewFromStreamRef.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
namespace NetVips.Samples
{
using System;
using System.IO;
/*using System.Threading.Tasks;*/

public class NewFromStreamRef : ISample
{
public string Name => "NewFromStream reference test";
public string Category => "Internal";

public const string Filename = "images/equus_quagga.jpg";

public void Execute(string[] args)
{
Cache.Max = 0;

/*Parallel.For(0, 1000, new ParallelOptions { MaxDegreeOfParallelism = NetVips.Concurrency },
i =>
{*/
using var stream = File.OpenRead(Filename);
var image = Image.NewFromStream(stream, access: Enums.Access.Sequential);

using var mutated = image.Mutate(mutable => mutable.Set(GValue.GStrType, "exif-ifd0-XPComment", "This is a test"));

Console.WriteLine($"Reference count image: {image.RefCount}");

// Test to ensure {Read,Seek}Delegate doesn't get disposed
image.Dispose();

Console.WriteLine($"Reference count mutated: {mutated.RefCount}");

var average = mutated.Avg();
Console.WriteLine($"Average: {average}");
/*});*/
}
}
}
56 changes: 40 additions & 16 deletions src/NetVips/GObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,16 @@ public class GObject : SafeHandle
/// We have to record all of the <see cref="SignalConnect"/> delegates to
/// prevent them from being re-located or disposed of by the garbage collector.
/// </summary>
/// <remarks>
/// All recorded delegates are freed in <see cref="ReleaseDelegates"/>.
/// </remarks>
private readonly ICollection<GCHandle> _handles = new List<GCHandle>();

/// <summary>
/// Hint of how much native memory is actually occupied by the object.
/// </summary>
internal long MemoryPressure;

/// <summary>
/// A delegate that is called when the object's reference count is decreased.
/// </summary>
internal event Action OnUnref;

// Handy for debugging
// public static int NObjects;

Expand Down Expand Up @@ -83,6 +81,15 @@ void EvalMarshal(IntPtr imagePtr, IntPtr progressPtr, IntPtr userDataPtr)
callback = (VipsImage.EvalSignal)EvalMarshal;
}

// add a weak reference callback to ensure all handles are released on finalization
if (_handles.Count == 0)
{
GWeakNotify notify = ReleaseDelegates;
var notifyHandle = GCHandle.Alloc(notify);

Internal.GObject.WeakRef(this, notify, GCHandle.ToIntPtr(notifyHandle));
}

// prevent the delegate from being re-located or disposed of by the garbage collector
var delegateHandle = GCHandle.Alloc(callback);
_handles.Add(delegateHandle);
Expand Down Expand Up @@ -125,23 +132,40 @@ protected override bool ReleaseHandle()
// logger.Debug($"Unref: GObject = {handle}");
if (!IsInvalid)
{
OnUnref?.Invoke();
Internal.GObject.Unref(handle);
}
// NObjects--;

return true;
}

// release all handles recorded by this object
foreach (var gcHandle in _handles)
/// <summary>
/// Release all the <see cref="SignalConnect"/> delegates by this object on finalization.
/// </summary>
/// <remarks>
/// This function is only called when <see cref="SignalConnect"/> was used on this object.
/// </remarks>
/// <param name="data">Data that was provided when the weak reference was established.</param>
/// <param name="objectPointer">The object being disposed.</param>
internal void ReleaseDelegates(IntPtr data, IntPtr objectPointer)
{
foreach (var gcHandle in _handles)
{
if (gcHandle.IsAllocated)
{
if (gcHandle.IsAllocated)
{
gcHandle.Free();
}
gcHandle.Free();
}

_handles.Clear();
}
// NObjects--;

return true;
// All GCHandles are free'd. Clear the list to prevent inadvertent use.
_handles.Clear();

// Free the GCHandle used by this GWeakNotify
var notifyHandle = GCHandle.FromIntPtr(data);
if (notifyHandle.IsAllocated)
{
notifyHandle.Free();
}
}

/// <summary>
Expand Down
Loading

0 comments on commit 8682c66

Please sign in to comment.