How to fix a CA2000 IDisposable C# compiler warning, when using a global cache

Question

CA2000 is a warning regarding the IDisposable interface:

CA2000 : Microsoft.Reliability : In method 'ImportProcessor.GetContext(string)', call System.IDisposable.Dispose on object 'c' before all references to it are out of scope.

My method is used to store a cache of context like so:

public class RegionContext : IDisposable { /* Implement Dispose() here */ } 

private Dictionary<string, RegionContext> contextCache = new ..... ();

public RegionContext GetContext(string regionCode)
{
    RegionContext rc = null;

    if (!this.contextCache.TryGetValue(regionCode.ToUpper(), out rc))
    {
        rc = new RegionContext(regionCode);
        this.contextCache.Add(regionCode.ToUpper(), rc);
    }

    return rc;
}

Where would you use the using() statement that fixes this compiler warning?

My outer class actually does iterate and dispose of the contents in the contextCache in its own implementation. Shall I suppress it, or is there a way to correctly get rid of this warning?

Answer 1

This CA2000 warning comes up any time you have a return value that's IDisposable and don't handle the case where the method throws an exception. In that instance, the caller won't get a valid instance of your object, so it has no way to dispose it. Therefore you have to.

I assume that you won't want to dispose the object if you pull it out of your cache successfully. In that case, you would need to do something like this to make sure the object you might create locally gets disposed in all cases:

public RegionContext GetContext(string regionCode)
{
    RegionContext temp = null;
    RegionContext rc = null;

    try
    {
        if (!this.contextCache.TryGetValue(regionCode.ToUpper(), out rc))
        {
            temp = new RegionContext(regionCode);
            this.contextCache.Add(regionCode.ToUpper(), temp);

            rc = temp;
            temp = null;
        }

        return rc;
    }
    finally 
    {
        if ( temp != null ) 
        {
             temp.Dispose();
        }
    }
}

Answer 2

What CA2000 is complaining about here is that the variable could be "orphaned" in an undisposed state if there's an exception while attempting to add it to the cache. To address the problem thoroughly, you could add a try/catch as follows (the newContext variable is used only so that CA2000 can detect the fix):

public RegionContext GetContext(string regionCode)
{
    RegionContext rc = null;
    if (!this.contextCache.TryGetValue(regionCode.ToUpper(), out rc))
    {
        RegionContext newContext = new RegionContext(regionCode);
        try
        {
            this.contextCache.Add(regionCode.ToUpper(), newContext);
        }
        catch
        {
            newContext.Dispose();
            throw;
        }

        rc = newContext;
    }

    return rc;
}

Personally, I find this sort of thing to be somewhat ridiculous overkill in most cases, but ymmv...

Answer 3

Michael's solution does not seem to work when converted to VB.Net. The following two functions were tested under VS 2017:

    Public Function OpenStream(ByVal filePathName As String) As System.IO.FileStream
        Dim fileStream As System.IO.FileStream = Nothing
        Dim tempFileStream As System.IO.FileStream = Nothing
        If Not String.IsNullOrWhiteSpace(filePathName) Then
            Try
                tempFileStream = New System.IO.FileStream(filePathName, System.IO.FileMode.Open, System.IO.FileAccess.Read)
                fileStream = tempFileStream
            Catch
                tempFileStream?.Dispose()
                Throw
            End Try
        End If
        Return fileStream
    End Function

    Public Function OpenReader(ByVal filePathName As String) As System.IO.BinaryReader
        If String.IsNullOrWhiteSpace(filePathName) Then Throw New ArgumentNullException(NameOf(filePathName))
        If Not System.IO.File.Exists(filePathName) Then Throw New System.IO.FileNotFoundException("Failed opening a binary reader -- file not found.", filePathName)
        Dim tempReader As System.IO.BinaryReader = Nothing
        Dim reader As System.IO.BinaryReader = Nothing
        Dim stream As IO.FileStream = Nothing
        Try
            stream = Methods.OpenStream(filePathName)
            tempReader = New System.IO.BinaryReader(stream)
            reader = tempReader
        Catch
            stream?.Dispose()
            tempReader?.Dispose()
            Throw
        End Try
        Return reader
    End Function