views:

2557

answers:

4

In Visual Studio, when I type the line "Implements IDisposable", the IDE automatically adds:

  • a disposedValue member variable
  • a Sub Dispose() Implements IDisposable.Dispose
  • a Sub Dispose(ByVal disposing As Boolean)

The Dispose() should be left alone, and the clean up code should be put in Dispose(disposing).

However the Dispose Finalize Pattern says you should also override Sub Finalize() to call Dispose(False). Why doesn't the IDE also add this? Must I add it myself, or is it somehow called implicitly?

EDIT: Any idea why the IDE automatically adds 80% of the required stuff but leaves out the Finalize method? Isn't the whole point of this kind of feature to help you not forget these things?

EDIT2: Thank you all for your excellent answers, this now makes perfect sense!

+6  A: 

If you actually are holding non-managed resources that will not be automatically cleaned up by the garbage collector and cleaning those up in your Dispose(), then yes, you should do the same in Finalize().

If you're implementing IDisposable for some other reason, implementing Finalize() isn't required.

The basic question is this: If Dispose() wasn't called and your object garbage collected, would memory leak? If yes, implement Finalize. If no, you don't need to. Also, avoid implementing Finalize "just because it's safer". Objects with custom finalizers can potentially need two GC passes to free them -- once to put them on the pending finalizers queue, and a second pass to actually free their memory.

Jonathan
Implementing Dispose does not mean you should also implement a finalizer. You can free unmanaged resources in a Dispose method without ever needing a finalizer. If you must implement a finalizer, your actual cleanup logic should be in a separate function that both dispose and finalize call.
Scott Dorman
That's incorrect. If you rely solely on Dispose() to free unmanaged resources, then memory will leak in cases where Dispose is not called. That's exactly what the finalizer is there for.
Laurent
Not sure why I didn't resond to Scott's comment earlier -- must not have noticed it, but Laurent is right -- if your Dispose is cleaning up unmanaged resources, a Finalize is required for safety. Don't leave it off because you're lazy.
Jonathan
+2  A: 

No, you don't need to have Finalize unless you have unmanaged resources to clean up.

In most cases the reason a class is disposable is because it keeps references to other managed IDisposable objects. In this case no Finalize method is necessary or desirable.

Matt Howells
You can free unmanaged resources in a Dispose method without ever needing a finalizer.
Scott Dorman
You can, but if Dispose is not called then you will probably leak memory without a Finalize method as well.
Matt Howells
+1  A: 

As others have said, you don't need to implement a finalizer unless you're directly holding unmanaged resources. Also, assuming you're working in .NET 2.0 or later, it's unlikely you'll ever need to implement a finalizer because typically SafeHandle can be used to wrap your unmanaged resources.

I wrote a fairly long blog post covering the background and implementation of IDisposable and finalizers a while back, which may be worth a read if you're not totally clear about it.

Greg Beech
You can free unmanaged resources in a Dispose method without ever needing a finalizer.
Scott Dorman
Well yes, that's the whole point of the IDisposable pattern. However if people forget to call Dispose then the finalizer is a safeguard that cleans them up eventually, which can't be forgotten about because it's called by the runtime.
Greg Beech
A: 
Implements IDisposable

Public Overloads Sub Dispose() Implements IDisposable.Dispose

    Dispose(True)
    GC.SuppressFinalize(Me)

End Sub

Protected Overloads Sub Dispose(ByVal disposing As Boolean)

    If disposing Then
        ' Free other state (managed objects).
    End If
    ' Free your own state (unmanaged objects).
    ' Set large fields to null.
End Sub

Protected Overrides Sub Finalize()

    Dispose(False)
    MyBase.Finalize()

End Sub
missa
A single slab of code isn't very useful. Try to be more constructive, with a small description and reasoning behind the provided code.
Shadow