The correct way would be not to write a Dispose function at all. At best you're just tying up the GC, preventing it from freeing your object until it invokes it on the GUI thread (slowing it down) and gets back confirmation that it has run. At worst you write it as bad as you did and end up with potential memory leaks.
Now if you handle native memory or resources directly in your object (and it's rare, you can avoid handling most of them and leave it to .NET), the correct way of writing a finalizer (ie the part you're missing!) is:
public class Base: IDisposable
{
private bool disposed = false;
//Implement IDisposable.
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
protected virtual void Dispose(bool disposing)
{
if (!disposed)
{
if (disposing)
{
// Free other state (managed objects).
}
// Free your own state (unmanaged objects).
// Set large fields to null.
disposed = true;
}
}
// Use C# destructor syntax for finalization code.
~Base()
{
// Simply call Dispose(false).
Dispose (false);
}
}
Where your code simply prevents finalizers from running, this code either calls Dispose(false) in your finalizer (ie when the GC determines that your object is useless and tries to free it) or Dispose(true) when you call Dispose() yourself (and then prevents the GC from calling the finalizer so you don't dispose twice).
As to how you use it, this is the correct way:
using(var obj = new Base())
{
// do whatever you want
// obj.Dispose() will automatically be called at the end of scope
}
Which is functionally equivalent to:
try
{
var obj = new Base();
// use the object
}
finally
{
// always called, even during exception stack unwinding
obj.Dispose();
}
And again, the correct way is not to write a Dispose. Every time you are required to do it, it's a failure of the framework to provide you with the right tools to stay in the managed world. These instances are fewer and fewer nowadays.
