Unit Testing WeakReference

Question

It seems the following code to unit test WeakReference does not work correctly/reliably:

object realObject = new object();
WeakReference weakReference = new WeakReference(realObject);
Assert.NotNull(weakReference.Target);
realObject = null;
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
Assert.Null(weakReference.Target);

The testing is run targeting both net461 and net5.0, for both DEBUG and RELEASE mode. The best result:

• Targeting net461: the above code runs successfully, for both DEBUG and RELEASE mode.

• Targeting net5.0: the above code fails for both DEBUG and RELEASE mode at first. After reading this post (Garbage collection behaviour difference between .NetFramework 4.8 and .Net 5) it works running in RELEASE mode by adding <TieredCompilation>false</TieredCompilation> to .csproj.

So the problem narrows down to targeting net5.0 DEBUG mode.

I also came across the following posts but it seems they're not very helpful mostly because they're out-of-date:

• Unit Testing Code with WeakReference
• Testing/Verifying a WeakReference

Worth to mention the answer from @Eric Lippert for Why C# Garbage Collection behavior differs for Release and Debug executables? says: You absolutely cannot rely on the garbage collector having any particular behaviour whatsoever with respect to the lifetime of a local, which suggests testing a WeakReference cannot be done reliably.

Edit: More information about the context of why using a WeakReference:

I'm developing a framework providing class Entity<T> and Record<T> similar to System.Data.DataRow and System.Data.DataRowView, with the most significance that the prior one is strongly typed. Entity is the model provides its own event to notify changes, Record<T> is a viewmodel that wraps Entity<T> to translate the change event into INotifyPropertyChanged plus other things such as databinding, validations, etc. Entity<T> should not aware the existence of Record<T>. To avoid memory leak, Record<T> should be GC reclaimed when only Entity is still referenced.

The example code for the event hookup:

IEntityListener Implemented by Record<T> class:

internal interface IEntityListener
{
    void OnFieldUpdated(FieldUpdatedEventArgs e);
}

WeakEntityListner to do the event hookup:

internal sealed class WeakEntityListener
{
    private readonly WeakReference<IEntityListener> _weakListener;
    private readonly Entity _entity;

    internal WeakEntityListener(Entity entity, IEntityListener listener)
    {
        _entity = entity;
        _weakListener = new (listener);
        _entity.FieldUpdated += OnFieldUpdated;
    }

    private void OnFieldUpdated(object? sender, FieldUpdatedEventArgs e)
    {
        if (_weakListener.TryGetTarget(out var listener))
            listener.OnFieldUpdated(e);
        else
            CleanUp();
    }

    private void CleanUp()
    {
        _entity.FieldUpdated -= OnFieldUpdated;
    }
}

I would like to have method OnFieldUpdated and CleanUp fully covered by unit test.

Answer 1

Now that you've explained the reason you are trying to use WeakReference. The solution you really need is to implement IDisposable. This gives the caller precise control of the lifetime of your classes, allowing the garbage collector to release resources as soon as they are unused.

I would consider the use of WeakReference to avoid IDisposable an anti-pattern. Garbage collection is an expensive operation. Each version of the .net runtime may tweak how the garbage collector works, how often it occurs, and what garbage is removed. Until that collection occurs, every instance of IEntityListener will continue to receive events after it's lifetime has been completed. Forcing any implementation to require extra guard code to prevent mis-behaving.

Though implementing IDisposable has an annoying tendency to spread throughout every class. The tradeoff is worth it. Even if you are building a framework for others to reuse. I don't believe that using IDisposable as part of the contract between client code and framework would dissuade anyone from using your library. However, attempting to use WeakReference to invert this contract, might give your library a negative reputation.

Answer 2

I ended up with the following solution, inspired by runtime WeakReference unit test source code suggested by @Jeremy Lakeman (https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime/tests/System/WeakReferenceTests.cs)

using System;
using Xunit;
using Xunit.Abstractions;

public class WeakReferenceTests
{
    private sealed class Latch
    {
        public bool FinalizerRan { get; set; }
    }

    private sealed class TestObject
    {
        ~TestObject()
        {
            Latch.FinalizerRan = true;
        }

        public Latch Latch { get; } = new();
    }

    private readonly ITestOutputHelper output;

    public WeakReferenceTests(ITestOutputHelper output)
    {
        this.output = output;
    }

    [Fact]
    public void Test1()
    {
        TestObject realObject = new TestObject();
        Latch l = realObject.Latch;
        Assert.False(l.FinalizerRan);
        var weakReference = new WeakReference(realObject);
        Assert.NotNull(weakReference.Target);
        GC.KeepAlive(realObject);
        realObject = null;
        GC.Collect();
        GC.WaitForPendingFinalizers();
        GC.Collect();
        if (!l.FinalizerRan)
            output.WriteLine("Attempted GC but could not force test object to finalize. Test skipped.");
        else
            Assert.Null(weakReference.Target);
    }
}

Answer 3

The reason this fails in DEBUG mode is explained in my answer to https://stackoverflow.com/a/58662332/1261844 (which you linked to). If you inspect the IL generated by your code, you will see that the compiler generated a reference to the object you created in the very first line in a temporary variable (and also possibly when you create the WeakReference). That temporary variable is rooting your object. When optimizations are turned on, the compiler generates different IL which avoids the use of temporary variables where possible.

Put simply, if you want to test something like this, you MUST create the object AND its WeakReference in another method. Not coincidentally, that's also what the code in your suggested answer (which is still wrong, btw) is doing.

Jermey Lakemen's answer is also correct. You shouldn't be using a finalizer to unhook an event. Managed resources (such as unhooking events) should always be cleaned up in a Dispose() method. Finalizers are for unmanaged resources. There's very real reasons for why using a finalizer to do this a terrible idea, not the least of which being that you can't guarantee that your class still has valid references inside of it when it's being finalized. Using a WeakEventManager is a last resort for when you don't know when an object will actually be free, and even then, that's usually a code smell that you are doing something wrong.