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?

Note I do not want to make it static, as I only want to cache the regions on a per-instance basis and not through multi-threading. — Dominic Zukiewicz, Jul 13 '11 at 19:06
FxCop isn't smart enough to see that you cache the object. And that you dispose it elsewhere, hopefully. You'll have to shut it up with an attribute. — Hans Passant, Jul 13 '11 at 19:08
Agree with Hans here. I've come across this quite a few times and it's best to simply add an attribute to supress it. — Bryan Crosby, Jul 13 '11 at 20:12

score 12 · Accepted Answer · answered Jul 14 '11 at 14:12

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

Ii could, but it doesn't matter. If the try block throws, it won't return anything, and if the try block doesn't throw, it will reach the return and return the right thing. The finally block will run either way. — Michael Edenfield, Nov 29 '14 at 17:40

score 6 · Answer 2 · answered Jul 14 '11 at 12:43

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

score 0 · Answer 3 · answered Apr 25 '17 at 18:10

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

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

3 Answers3

Linked

Related