nsubstitute/NSubstitute

NSubstitute makes it impossible to unit test memory leaks

albyrock87 opened this issue · 4 comments

Describe the bug
I'm trying to test memory leaks on my library, but NSubstitute retains reference to call arguments (even after ClearReceivedCalls).

This makes it impossible to test memory leaks which in some part require mocked interfaces.

To Reproduce

The following unit test fails.

public class NSubstituteLeakTests
{
    public class Foo;

    public interface IInterface
    {
        public void Method(Foo foo);
    }

    private readonly IInterface _interfaceMock = Substitute.For<IInterface>();

    [Fact(DisplayName = "NSubstitute does not leak")]
    public async Task NSubstituteDoesNotLeak()
    {
        WeakReference<Foo> weakFoo;
        {
            var foo = new Foo();
            weakFoo = new WeakReference<Foo>(foo);
            _interfaceMock.Method(foo);
            _interfaceMock.ClearReceivedCalls();
        }

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();

        weakFoo.TryGetTarget(out _).Should().BeFalse();
    }
}

This is what retains the reference to the object.
image

Expected behaviour
Unit test above passes.

Environment:

  • NSubstitute version: 5.1.0
  • NSubstitute.Analyzers version: 1.0.16
  • Platform: .NET 8 on MacBook M3

@albyrock87 Thank you for the issue and great repro!
I tried to remove NSubstitute related code from your test but it still fails. I don't think it's NSubstitute related.

public class NSubstituteLeakTests
{
    public class Foo;

    [Fact(DisplayName = "NSubstitute does not leak")]
    public async Task NSubstituteDoesNotLeak()
    {
        WeakReference<Foo> weakFoo;
        {
            var foo = new Foo();
            weakFoo = new WeakReference<Foo>(foo);
        }

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();

        weakFoo.TryGetTarget(out _).Should().BeFalse();
    }
}

@alexandrnikitin I was pretty sure about the issue so I double checked again, and as you can see from this screenshot, the test without NSubstitute is working (see the green mark on the left side)

image

This makes me think we're running the unit test on different frameworks.

My XUnit test project is using net8.0 as TargetFramework and I have .NET 8.0.101 installed.

    <PackageReference Include="xunit" Version="2.4.2" />
    <PackageReference Include="xunit.runner.visualstudio" PrivateAssets="all" Version="2.4.5">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>

Here's a test that fails for me on Windows and .NET 8 #775
Not sure what is the difference in our setup but I don't think WeakReference is deterministic enough for memory leak tests.

@alexandrnikitin this is what Microsoft does to test leaks

Anyway let's try with this:

public class NSubstituteLeakTests
{
    public class Foo;

    public interface IInterface
    {
        public void Method(Foo foo);
    }

    // private readonly IInterface _interfaceMock = Substitute.For<IInterface>();
    [Fact(DisplayName = "NSubstitute does not leak")]
    public async Task NSubstituteDoesNotLeak()
    {
        WeakReference<Foo> weakFoo;
        {
            var foo = new Foo();
            weakFoo = new WeakReference<Foo>(foo);

            // _interfaceMock.Method(foo);
            // _interfaceMock.ClearReceivedCalls();
        }

        await GcCollect();

        weakFoo.TryGetTarget(out _).Should().BeFalse();
    }

    private static async Task GcCollect()
    {
        for (var i = 0; i < 3; i++)
        {
            await Task.Yield();
            GC.Collect();
            GC.WaitForPendingFinalizers();
        }
    }
}