7

I have a class that implements IDisposable because it has a private member field "foo" that is IDisposable (which is initialized in the constructor). I am unexpectedly getting a CA2000 Code Analysis error, which wants me to be sure to dispose of foo. However, I have foo.Dispose() in my class's Dispose() code, which should take care of this.

I did some searching around and surprisingly can't find an answer. What am I doing wrong? Clearly I missing something basic. How do I write my code to overcome this?

My VB code:

Public Class Bar
    Implements IDisposable

    Private Foo As SomeDisposableThing

    Public Sub New()
        Foo = New SomeDisposableThing() With {.name = "hello"}
    End Sub

    '''' snip ''''

    Private disposedValue As Boolean = False        ' To detect redundant calls '

    Protected Overridable Sub Dispose(ByVal disposing As Boolean)
        If Not Me.disposedValue Then
            If disposing Then
                If Foo IsNot Nothing Then Foo.Dispose()
            End If
        End If
        Me.disposedValue = True
    End Sub

    Public Sub Dispose() Implements IDisposable.Dispose
        Dispose(True)
        GC.SuppressFinalize(Me)
    End Sub

End Class
Dominic Zukiewicz
  • 8,258
  • 8
  • 43
  • 61
Patrick Szalapski
  • 8,738
  • 11
  • 67
  • 129

1 Answers1

7

The CA2000 error doesn't refer to the container implementing IDisposable but rather the use of a local which is not properly disposed. The reason why is that you are using an object initializer on the disposable object. The actual code that will get generated is essentially the following

Dim temp = New SomethingDisposable()
temp.Name = "hello"
Foo = temp

This code is correctly flagged by FxCop as not properly disposing of an IDisposable in all instances (it's possible for an exception to occur on the temp.Name = "hello" line in which case it wouldn't be disposed).

The fix is to not use an object initializer here and initialize Foo directly

Foo = New SomethingDisposable()
Foo.Name = "hello"
JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
  • Yuck. Message: don't use a property :( – Hans Passant Jan 12 '11 at 00:28
  • @Hans, I think it will have the same behavior even if it's a field. Any line can potentially throw an exception (`ThreadAbort` for example) so I would *guess* that FxCop would warn equally for fields and properties here. Haven't tested that theory yet though. – JaredPar Jan 12 '11 at 00:39
  • @Hans, interesting. I guess it only flags method / prop calls as dangerous then. – JaredPar Jan 12 '11 at 00:56
  • Yuck indeed! The "With" initializer is supposed to abstract this away. I shouldn't have to know that the "With" initializer uses a temp variable! – Patrick Szalapski Jan 12 '11 at 02:57