views:

862

answers:

8

I've got a C# class with a Dispose function via IDisposable. Its intended to be used inside a using block so the expensive resource it handles can be released right away.

Problem is that a bug occurred when an exception was thrown before Dispose was called, and the programmer neglected to use using or finally.

<rant>
In C++, I never had to worry about this. The call to a class's destructor would be automatically inserted at the end of the object's scope. The only way to avoid that happening would be to use the new operator and hold the object behind a pointer, but that required extra work for the programmer isn't something they would do by accident, like forgetting to use using.
</rant>

Is there anyway to for a using block to be automatically used in C#?

Many thanks.

UPDATE:

I'd like to explain why I'm not accepting the finalizer answers. Those answers are technically correct in themselves, but they are not C++ style destructors.

Here's the bug I found, reduced to the essentials...

try
{
    PleaseDisposeMe a = new PleaseDisposeMe();
    throw new Exception();
    a.Dispose();
}
catch (Exception ex)
{
    Log(ex);
}

// This next call will throw a time-out exception unless the GC
// runs a.Dispose in time.
PleaseDisposeMe b = new PleaseDisposeMe();

Using FXCop is an excellent suggestion, but if that's my only answer, my question would have to become a plea to the C# people, or use C++. Twenty nested using statements anyone?

+1  A: 
~ClassName()
{
}

EDIT (bold):

If will get called when the object is moved out of scope and is tidied by the garbage collector however this is not deterministic and is not guaranteed to happen at any particular time. This is called a Finalizer. All objects with a finaliser get put on a special finalise queue by the garbage collector where the finalise method is invoked on them (so it's technically a performance hit to declare empty finalisers).

The "accepted" dispose pattern as per the Framework Guidelines is as follows with unmanaged resources:

 public class DisposableFinalisableClass : IDisposable
 {
  ~DisposableFinalisableClass()
  {
   Dispose(false);
  }

  public void Dispose()
  {
   Dispose(true);
  }

  protected virtual void Dispose(bool disposing)
  {
   if (disposing)
   {
    // tidy managed resources
   }

   // tidy unmanaged resources
  }
 }

So the above means that if someone calls Dispose the unmanaged resources are tidied. However in the case of someone forgetting to call Dispose or an exception preventing Dispose from being called the unmanaged resources will still be tidied away, only slightly later on when the GC gets its grubby mitts on it (which includes the application closing down or unexpectedly ending).

Quibblesome
+1  A: 

Unfortunately there isn't any way to do this directly in the code. If this is an issue in house, there are various code analysis solutions that could catch these sort of problems. Have you looked into FxCop? I think that this will catch these situations and in all cases where IDisposable objects might be left hanging. If it is a component that people are using outside of your organization and you can't require FxCop, then documentation is really your only recourse :).

Edit: In the case of finalizers, this doesn't really guarantee when the finalization will happen. So this may be a solution for you but it depends on the situation.

Ben Childs
A: 

This is no different from a programmer forgetting to use delete in C++, except that at least here the garbage collector will still eventually catch up with it.

And you never need to use IDisposable if the only resource you're worried about is memory. The framework will handle that on it's own. IDisposable is only for unmanaged resources like database connections, filestreams, sockets, and the like.

Joel Coehoorn
"IDisposable is only for unmanaged resources..."What if I write a class that subscribes to an event in its constructor? I need a corresponding place where I can unsubscribe, and clients need a standard way of asking me to clean up such things. Otherwise my object may be kept alive indefinitely.
Daniel Earwicker
In that case, your contention is that events fall into the 'unmanaged' category.
Joel Coehoorn
Joel->Wrong - If you have an unmanaged object that needs or uses memory then you must be careful with you dispose pattern and your use of memory pressure. Ignoring it will lead to either: unmanaged resource hogs memory that the gc would like or unmanaged allocation fails because GC hogs memory.
morechilli
+1  A: 

@Quarrelsome

If will get called when the object is moved out of scope and is tidied by the garbage collector.

This statement is misleading and how I read it incorrect: There is absolutely no guarantee when the finalizer will be called. You are absolutely correct that billpg should implement a finalizer; however it will not be called automaticly when the object goes out of scope like he wants. Evidence, the first bullet point under Finalize operations have the following limitations.

In fact Microsoft gave a grant to Chris Sells to create an implementation of .NET that used reference counting instead of garbage collection Link. As it turned out there was a considerable performance hit.

Andrew Burns
A: 

A better design is to make this class release the expensive resource on its own, before its disposed.

For example, If its a database connection, only connect when needed and release immediately, long before the actual class gets disposed.

FlySwat
A: 

@Joel Coehoorn

forgetting to use delete in C++

Granted, but for that to be an issue, the programmer would have to call new as well. They won't accidentally do that.

(Or will they?)

billpg
+4  A: 

Where I work we use the following guidelines:

  • Each IDisposable class must have a finalizer
  • Whenever using an IDisposable object, it must be used inside a "using" block. The only exception is if the object is a member of another class, in which case the containing class must be IDisposable and must call the member's 'Dispose' method in its own implementation of 'Dispose'. This means 'Dispose' should never be called by the developer except for inside another 'Dispose' method, eliminating the bug described in the question.
  • The code in each Finalizer must begin with a warning/error log notifying us that the finalizer has been called. This way you have an extremely good chance of spotting such bugs as described above before releasing the code, plus it might be a hint for bugs occuring in your system.

To make our lives easier, we also have a SafeDispose method in our infrastructure, which calls the the Dispose method of its argument within a try-catch block (with error logging), just in case (although Dispose methods are not supposed to throw exceptions).

See also: Chris Lyon's suggestions regarding IDisposable

Edit: @Quarrelsome: One thing you ought to do is call GC.SuppressFinalize inside 'Dispose', so that if the object was disposed, it wouldn't be "re-disposed".

It is also usually advisable to hold a flag indicating whether the object has already been disposed or not. The follwoing pattern is usually pretty good:

class MyDisposable: IDisposable {
    public void Dispose() {
        lock(this) {
            if (disposed) {
                return;
            }

            disposed = true;
        }

        GC.SuppressFinalize(this);

        // Do actual disposing here ...
    }

    private bool disposed = false;
}

Of course, locking is not always necessary, but if you're not sure if your class would be used in a multi-threaded environment or not, it is advisable to keep it.

Hershi
Adding finalizers to every object that implement IDisposable just so you can log when the finalizer has been called is not a good idea. Finalizers add a cost to your object (it survives an extra GC cycle) and most of the assumptions you can make about internal state aren't valid.
Scott Dorman
"Where I work we use the following guidelines: Each IDisposable class must have a finalizer..."Change your guidelines.
Daniel Earwicker
A: 

The best practice is to use a finaliser in your class and always use using blocks.

There isn't really a direct equivalent though, finalisers look like C destructors, but behave differently.

You're supposed to nest using blocks, that's why the C# code layout defaults to putting them on the same line...

using (SqlConnection con = new SqlConnection("DB con str") )
using (SqlCommand com = new SqlCommand( con, "sql query") )
{
    //now code is indented one level
    //technically we're nested twice
}

When you're not using using you can just do what it does under the hood anyway:

PleaseDisposeMe a;
try
{
    a = new PleaseDisposeMe();
    throw new Exception();
}
catch (Exception ex) { Log(ex); }  
finally {    
    //this always executes, even with the exception
    a.Dispose(); 
}

With managed code C# is very very good at looking after its own memory, even when stuff is poorly disposed. If you're dealing with unmanaged resources a lot it's not so strong.

Keith