Can a CodeAnalysis return a false positive of CA2202? or is really something wrong with my code?

Question

I'm suffering the same issue explained here but iterating a EnvDTE.Processes.

In the question that I linked the user @Plutonix affirms it is a false warning, and I think him reffers to the obj.Getenumerator() mention so I assume my problem will be considered a false warning too, however, if this is a false warning I would like to know more than an affirmation, the arguments to say it is a false warning.

This is the warning:

CA2202 Do not dispose objects multiple times Object 'procs.GetEnumerator()' can be disposed more than once in method 'DebugUtil.GetCurrentVisualStudioInstance()'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 214 Elektro.Application.Debugging DebugUtil.vb 214

This is the code, procs object is the involved one on the warning, but I don't see any disposable object:

Public Shared Function GetCurrentVisualStudioInstance() As DTE2

    Dim currentInstance As DTE2 = Nothing
    Dim processName As String = Process.GetCurrentProcess.MainModule.FileName
    Dim instances As IEnumerable(Of DTE2) = DebugUtil.GetVisualStudioInstances
    Dim procs As EnvDTE.Processes

    For Each instance As DTE2 In instances

        procs = instance.Debugger.DebuggedProcesses

        For Each p As EnvDTE.Process In procs

            If (p.Name = processName) Then
                currentInstance = instance
                Exit For
            End If

        Next p

    Next instance

    Return currentInstance

End Function

PS: Note that the code-block depends on other members but they are unrelevant for this question.

Answer 1

Short version: this looks like a bug in the Code Analysis component to me.

Long version (hey, you suckered me into spending the better part of my afternoon and evening deciphering this, so you might as well spend a little time reading about it :) )…

The first thing I did was look at the IL. Contrary to my guess, it did not contain multiple calls to Dispose() on the same object. So much for that theory.

The method did, however, contain two separate calls to Dispose(), just on different objects. By this time, I was already convinced this was a bug. I've seen mention of CA2202 being triggered when dealing with related classes where one class instance "owns" an instance of the other class, and both instances are disposed. While inconvenient and worth suppressing, the warning seems valid in those cases; one of the objects really is getting disposed of twice.

But in this case, I had two separate IEnumerator objects; one did not own, nor was even related to, the other. Disposing one would not dispose the other. Thus, Code Analysis was wrong to warn about it. But what specifically was confusing it?

After much experimentation, I came up with this near-minimal code example:

Public Class A
    Public ReadOnly Property B As B
        Get
            Return New B
        End Get
    End Property
End Class

Public Interface IB
    Function GetEnumerator() As IEnumerator
End Interface

Public Class B : Implements IB
    Public Iterator Function GetEnumerator() As IEnumerator Implements IB.GetEnumerator
        Yield New C
    End Function
End Class

Public Class C
    Dim _value As String
    Public Property Value As String
        Get
            Return _value
        End Get

        Set(value As String)
            _value = value
        End Set
    End Property
End Class

Public Shared Function GetCurrentVisualStudioInstance2() As A
    For Each a As A In GetAs()
        For Each c As C In a.B
            If (c.Value = Nothing) Then
                Return a
            End If
        Next c
    Next a

    Return Nothing
End Function

Public Shared Iterator Function GetAs() As IEnumerable(Of A)
    Yield New A()
End Function

This produces the same spurious CA2202 you are seeing in the other code example. Interestingly though, a minor change to the declaration and implementation of interface IB causes the warning to go away:

Public Interface IB : Inherits IEnumerable
End Interface

Public Class B : Implements IB
    Public Iterator Function GetEnumerator() As IEnumerator Implements IEnumerable.GetEnumerator
        Yield New C
    End Function
End Class

Somehow, Code Analysis is getting confused by the non-IEnumerable implementation of GetEnumerator(). (Even more weirdly is that the actual type you're using, the Processes interface in the DTE API, both inherits IEnumerable and declares its own GetEnumerator() method…but it's the latter that is the root of the confusion for Code Analysis, not the combination).

With that in hand, I tried to reproduce the issue in C# and found that I could not. I wrote a C# version that was structured exactly as the types and methods in the VB.NET version, but it passed Code Analysis without warnings. So I looked at the IL again.

I found that the C# compiler generates code very similar to, but not exactly the same as, the VB.NET compiler. In particular, for the try/finally blocks that protect the IEnumerator returned for each loop, all of the initialization for those loops is performed outside the try block, while in the VB.NET version the initialization is performed inside.

And apparently, that is also enough to prevent Code Analysis from getting confused about the usage of the disposable objects.

Given that it seems to be the combination of VB.NET's implementation of For Each and the nested loops, one work-around would be to just implement the method differently. I prefer LINQ syntax anyway, and here is a LINQified version of your method that compiles without the Code Analysis warning:

Public Shared Function GetCurrentVisualStudioInstance() As DTE2
    Dim processName As String = Process.GetCurrentProcess.MainModule.FileName

    Return GetVisualStudioInstances.FirstOrDefault(
        Function(instance)
            Return instance.Debugger.DebuggedProcesses.Cast(Of EnvDTE.Process).Any(
                Function(p)
                    Return p.Name = processName
                End Function)
        End Function)
End Function

And for completeness, the C# version (since all of this code started when a C# implementation was converted to VB.NET and then extended to handle the "current instance" case):

public static DTE2 GetCurrentVisualStudioInstance()
{
    string processName = Process.GetCurrentProcess().MainModule.FileName;

    return GetVisualStudioInstances()
        .FirstOrDefault(i => i.Debugger.DebuggedProcesses
            .Cast<EnvDTE.Process>().Any(p => p.Name == processName));
}