IDisposable and the New Keyword

Introduction

It’s impossible to look at a code snippet taken out of context and say it contains bugs without knowing the full intention behind it. Even Application.DoEvents() can be called safely if the author of the code is fully aware of its effects. The best you can do is to hypothesize about what the code is supposed to do, and spot potential bugs based on that hypothesis. However, this snippet is almost certainly wrong. At best, it’s a bug in hibernation, waiting to be awoken by some poor unsuspecting developer’s check in:

// Assuming BaseDisposable : IDisposable
class MyDisposable : BaseDisposable
{
public void new Dispose()
{
// Implementation irrelevant.
}
}

Potential Badness

As you probably know, IDisposable is implemented when your class owns unmanaged resources, or owns instances of other managed classes which implement IDisposable. It signifies to a consumer that instances of the class use resources which should be explicitly released.

Typically, IDisposable.Dispose is implemented as non-virtual, so it can’t be overriden. Instead, a virtual Dispose(Boolean) method is provided for override in subclasses. The Boolean parameter represents whether the call is coming from user code, or from the finalizer. The standard job of the non-virtual IDisposable.Dispose() method is to call the virtual Dispose(Boolean) and supresses finalization of the object.

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
~T()
{
Dispose(false);
}
public virtual void Dispose(Boolean disposing)
{
if (disposing)
{
// Clean up any IDisposables we own.
}
// Release unmanaged resources we own.
}

So why is that code snippet so bad then?

Looking at the public void new Dispose() method, there are a couple of reasons it could have been written that way:

BaseDisposable implements the Dispose pattern, but we’ve incorrectly provided a new implementation for Dispose() rather than Dispose(Boolean).
BaseDisposable does not implement the Dispose pattern, and its Dispose method is non-virtual.

The first reason is obviously a bad one. Dispose(Boolean) should be overriden.

If we have no control over our base class, it may seem that the second reason is a decent enough excuse to write the code like that. However, there’s actually a pretty big bug in the code involving the use of the new keyword.

Virtual Versus New

C# allows non-virtual methods to be ‘overriden’ if the new keyword is specified. There is a major difference however between a virtual and non-virtual override.

A virtual override using the override keyword is called no matter what the compile time type of an instance of the class is.
A non-virtual override using the new keyword is only called if the compile time type is the same as the type defining the override.

Basically, if we were to define the following:

class Base
{
public virtual String GetMessage()
{
return "Hello";
}
}
class Sub : Base
{
public override String GetMessage()
{
return base.GetMessage() + " Alex";
}
}
static void Main(String[] args)
{
var s = new Sub();
Console.WriteLine(s.GetMessage());
var b = (Base)s;
Console.WriteLine(b.GetMessage());
}

We’d get the following output:

Hello Alex
Hello Alex

This is because the GetMessage method is resolved using the runtime-type of s and b which is Sub in both cases. If we change the code to:

class Base
{
public String GetMessage()
{
return "Hello";
}
}
class Sub : Base
{
public new String GetMessage()
{
return base.GetMessage() + " Alex";
}
}
static void Main(String[] args)
{
var s = new Sub();
Console.WriteLine(s.GetMessage());
var b = (Base)s;
Console.WriteLine(b.GetMessage());
}

We’d get the following instead:

Hello Alex
Hello

The GetMessage method is now resolved using the compile-time types of s and b, which are Sub and Base respectively.

So?

This means that our new Dispose() method will only be called if the compile time type is MyDisposable. If the compile time type is BaseDisposable or IDisposable, the new override will not be called – the Dispose() defined by BaseDisposable will be called.

Suppose MyDisposable adds another file which must be cleaned up by Dispose. We add the cleanup code to our new Dispose method. The following code will fare OK, with the file being correctly cleaned up:

var myDisp = new MyDisposable();
try
{
// Something…
}
finally
{
myDisp.Dispose();
}

However, the code next will not clean up the file, because BaseDisposable.Dispose() will be called, not MyDisposable.Dispose():

BaseDisposable myDisp = new MyDisposable();
try
{
// Something…
}
finally
{
myDisp.Dispose();
}

Now I admit, that’s a fairly contrived example. However, it’s easy to imagine more complicated real world examples where our disposable is perhaps stored in a collection of IDisposables. That doesn’t really matter though, because it gets worse. Any guesses for what this type will do when used in the extremely commonly used using block?

using (var myDisp = new MyDisposable())
{
// Something…
}

What Should it Do?

It appears that because the compile time type of myDisp is MyDisposable, MyDisposable.Dispose will be called and all will be fine. What actually happens is that BaseDisposable.Dispose is called. If we refer to section 8.13 of the C# 4.0 specification, we can see why:

A using statement of the form

using (ResourceType resource = expression) statement

corresponds to one of three possible expansions.[…]When ResourceType is a nullable value type or a reference type other than dynamic, the expansion is

{
ResourceType resource = expression;
try {
statement;
}
finally {
if (resource != null) ((IDisposable)resource).Dispose();
}
}

We can see that the disposable object is explicitly cast to an instance of IDisposable in a using statement. This is done on purpose in order to avoid calling new implementations. In our example, BaseDisposable.Dispose gets called as opposed to MyDisposable.Dispose. Any resources MyDisposable has to clean up are left around.

Conclusions

We can see that MyDisposable is a pretty defective type. That small snippet turned out to give us a very badly behaved class. To be honest, whenever I see the new keyword on a method declaration, I always try to investigate why it’s being used. In fact, I don’t think I’ve ever required different methods to be called based on the compile time type of an object.

Most of the time I’ve seen the new keyword used, it’s been used to hide compiler warnings about method hiding without knowing what it actually means. I tend not to see it often, but when I do, alarm bells start ringing. If you see public void new Dispose(), you’ve probably got a bug on your hands.

Share and Enjoy:

nf₂p