Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NSubstitute makes it impossible to unit test memory leaks #771

Open
albyrock87 opened this issue Jan 28, 2024 · 4 comments
Open

NSubstitute makes it impossible to unit test memory leaks #771

albyrock87 opened this issue Jan 28, 2024 · 4 comments

Comments

@albyrock87
Copy link

albyrock87 commented Jan 28, 2024

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
@alexandrnikitin
Copy link
Member

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

@albyrock87
Copy link
Author

albyrock87 commented Jan 29, 2024

@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>

@alexandrnikitin
Copy link
Member

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.

@albyrock87
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants