False positive "Pulse Resource Leak" when storing data in collections

[crener]

Found the core issue in a real codebase and made this minimal repo case from it so that this is more convenient to reproduce

public class Program : IDisposable
{
    public static void Main(string[] args)
    {
        using Program program = new Program();
        program.Init();
    }

    private readonly Reader?[] m_objects = new Reader[1]; // same issue when using a List<Reader>

    public void Init()
    {
        // if we store this directly (not as the single object in the array) infer-sharp is happy
        m_objects[0] = new Reader();
    }

    public void Dispose()
    {
        m_objects[0]?.Dispose();
        m_objects[0] = null;
    }
}

public class Reader : IDisposable
{
    public virtual void Dispose() { }
}

Program.cs:7: error: Pulse Resource Leak
  Resource dynamically allocated by constructor Reader(), indirectly via call to `Program.Init()` on line 6 is not closed after the last access at line 7, column 2.

I think that Infer-sharp is loosing the reference when it get's stored in the m_objects array. Changing m_objects from Reader[] to List<Reader> doesn't make a difference and storing as an IDisposable rather than the Reader type also doesn't matter. The Reader is just there to represent a Disposable, in real life this would be something more important like a StreamReader.

Am I just missing something obvious or is this a bug in the C# translation layer?

[matjin]

There are some false positives for resource leaks like this (and some others) as well as for null dereferences (i.e. some kinds of Assert statements) which still occur and which I will address in the next release. It has to do with how certain language specific libraries/objects are modeled by the Infer analysis. Thanks for raising this!

[christopher-watanabe-snkeos]

I'm also getting a false positive for an IDisposable implementation. I use the action to incorporate analysis into my Git CI. I've been trying to avoid the analysis on the file hosting the affected code, while also allowing the action to continue failing on finding any other issues. For this, I give the following flags in the action's optional-flags input:

--skip-analysis-in-path '.*my-filename.*' --fail-on-issue

Git action logs show the following:

Run microsoft/infersharpaction@v1.5
  with:
    binary-path: memory-leak-build
    optional-flags: --skip-analysis-in-path .*my-filename.* --fail-on-issue
    github-sarif: false
  env:
    DOTNET_ROOT: /usr/share/dotnet

Interestingly, the file continues to be pulled into the analysis and upon analysis, the false-positive fails my CI. I'm at a loss for why this is happening. Any advice?

[matjin]

Hi Christopher,

I'm not too surprised that this isn't working for you given that Infer# works on compiled DLLs/PDBs, as opposed to Java for example where Infer is run as part of the build from the source code. I think I need to hack this a bit to get this to work for .NET, which hopefully is not too hard.

As a short term mitigation, one thing you could do if it isn't too aggressive is to just exclude the DLL which the file was built into. This would of course also potentially create some false negatives because you would exclude analysis of more than just the file, but it can be a short-term mitigation while I look into a better solution for you.

Matthew

[matjin]

Hi Christopher,

I just wanted to update you on this topic. Recently a new flag was added to Infer that enables you to pass an offline list of issues that you want to suppress:

https://github.com/facebook/infer/commit/504b8077e87fc3bd7c35c9aac406de2473982fa4

Does this look like it would address your situation? If so I can pull the latest binaries and create an updated release.

Matthew