DiagnosticsCollector provokes memory leaks

[Maesla]

I was having a strange behavior with some instances not destroyed and some memory leaks.
Then I realized that this behavior was only occurring when VContainerSettings.DiagnosticsEnabled = true.

I have done a Test trying to isolate and simplify my case and indeed, aparrently the Diagnostics is creating some memory leak, not allowing the gc to destroy some instances

using NUnit.Framework;
using System;
using System.Collections;
using UnityEngine;
using UnityEngine.TestTools;
using VContainer;
using VContainer.Diagnostics;

public class MemoryLeakTests
{
    private class DestroyCount
    {
        public int Count { get; private set; }

        public DestroyCount()
        {
            Count = 0;
        }

        public void IncreaseCount(string msg)
        {
            UnityEngine.Debug.Log(msg);
            Count++;
        }
    }


    private interface IFoo{}

    private class  FooA : IFoo
    {
        private readonly DestroyCount destroyCount;

        public FooA(DestroyCount destroyCount)
        {
            this.destroyCount = destroyCount;
        }

        ~FooA()
        {
            destroyCount.IncreaseCount("FooA destroyed");
        }
    }

    private class FooB : IFoo
    {
        private readonly DestroyCount destroyCount;

        public FooB(DestroyCount destroyCount)
        {
            this.destroyCount = destroyCount;
        }

        ~FooB()
        {
            destroyCount.IncreaseCount("FooB destroyed");
        }
    }

    private class FooFactory
    {
        public IFoo foo { get; set; }
    }


    static bool[] diagnosticsValues = new bool[] { true, false};

    [UnityTest]
    public IEnumerator MemoryLeakWithDiagnosisTest([ValueSource(nameof(diagnosticsValues))] bool useDiagnostics)
    {
        DestroyCount destroyCount = new DestroyCount();

        var builder = new ContainerBuilder();
        DiagnosticsCollector diagnosticsCollector = useDiagnostics ? DiagnositcsContext.GetCollector("test") : null;
        builder.Diagnostics = diagnosticsCollector;

        builder.Register<FooFactory>(Lifetime.Singleton);
        builder.Register<IFoo>(container => container.Resolve<FooFactory>().foo, Lifetime.Transient);

        var resolver = builder.Build();

        {
            FooFactory fooFactory = resolver.Resolve<FooFactory>();
            fooFactory.foo = new FooA(destroyCount);
        }
        {
            IFoo fooA = resolver.Resolve<IFoo>();
            Assert.That(fooA, Is.InstanceOf<FooA>());
        }
        {
            FooFactory fooFactory = resolver.Resolve<FooFactory>();
            fooFactory.foo = new FooB(destroyCount);
        }
        { 
            IFoo fooB = resolver.Resolve<IFoo>();
            Assert.That(fooB, Is.InstanceOf<FooB>());
        }

        //uncomment this line to pass both tests
        //diagnosticsCollector?.Clear(); 

        GC.Collect();

        yield return new WaitForSeconds(20f);
        Assert.That(destroyCount.Count, Is.EqualTo(1));
    }

    [UnityTest]
    public IEnumerator MemoryLeakGroundTruth()
    {
        DestroyCount destroyCount = new DestroyCount();

        FooFactory  fooFactory = new FooFactory();
        
        fooFactory.foo = CreateFooA(destroyCount);
        fooFactory.foo = CreateFooB(destroyCount);

        GC.Collect();

        yield return new WaitForSeconds(10f);
        Assert.That(destroyCount.Count, Is.EqualTo(1));
    }

    private IFoo CreateFooA(DestroyCount destroyCount)
    {
        return new FooA(destroyCount);
    }

    private IFoo CreateFooB(DestroyCount destroyCount)
    {
        return new FooB(destroyCount);
    }
}

[hadashiA]

In above test case, you have not Dispose the resolver ( var resolver = builder.Build();), so all instances may be retained, not just DiagnosticsCollector.

If you call GC.Collect(), is there any guarantee that all finalisers will be executed in the line immediately following it?

The only way to be sure is to Dispose.
If LifetimeScope is used, it will be DIsposed if you Destroy.
Are there any other problematic cases, other than this test case, on which Dispose is executed?

[Maesla]

It is not that DiagnosticsCollector, it's that the DiagnosticsCollector is preventing the gc to collect instances that are not referenced anymore, but probably the diagnostics is keeping some reference to them.

Maybe it wasn't clear in the test.
In MemoryLeakWithDiagnosisTest, new FooA is created outside VContainer
(fooFactory.foo = new FooA(destroyCount);),

but it is resolved using a factory
....
builder.Register(container => container.Resolve().foo, Lifetime.Transient);
......
IFoo fooA = resolver.Resolve();

Then, this object is unreferenced everywhere, because other new object has been created
fooFactory.foo = new FooB(destroyCount);

So, if it is unreferenced everywhere, if you call the gc, this object should be collected and cleaned at some point. You can see exactly that behaviour in the test MemoryLeakGroundTruth, that have this logic but not using VContainer.

My point is that the gc is not able to clean this unreferenced object because the diagnosticsCollector is keeping track of it, although is not been using anymore.

And you can see that this is true because the same test, with diagnosticsCollector = null, the object is correctly collected.
You can check also that clearing the diagnosticsCollector diagnosticsCollector?.Clear() allows the gc to correctly collect all the unreferenced objects

This was only a toy example to show this issue, but I had it in a real logic, where a type was resolved using a reference in a factory. It had a problem because I was overwritting the object that need to be used in the resolve, and the old instances weren't never collected by the GC. But this happened only with the Diagnistic On. If I wasn't using the diagnostic, the gc collector worked as expected.