Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should you implement IDisposable.Dispose() so that it never throws?

People also ask

What happens if you dont Dispose IDisposable?

IDisposable is usually used when a class has some expensive or unmanaged resources allocated which need to be released after their usage. Not disposing an object can lead to memory leaks.

Why should we implement IDisposable?

in a class, you should implement IDisposable and overwrite the Dispose method to allow you to control when the memory is freed. If not, this responsibility is left to the garbage collector to free the memory when the object containing the unmanaged resources is finalized.

How do you Dispose of IDisposable?

If you don't use using , then it's up to you (the calling code) to dispose of your object by explicitely calling Dispose().

When should you use IDisposable?

You should implement IDisposable when your class holds resources that you want to release when you are finished using them. Show activity on this post. When your class contains unmanaged objects, resources, opened files or database objects, you need to implement IDisposable .


The Framework Design Guidelines (2nd ed) has this as (§9.4.1):

AVOID throwing an exception from within Dispose(bool) except under critical situations where the containing process has been corrupted (leaks, inconsistent shared state, etc.).

Commentary [Edit]:

  • There are guidelines, not hard rules. And this is an "AVOID" not a "DO NOT" guideline. As noted (in comments) the Framework breaks this (and other) guidelines in places. The trick is knowing when to break a guideline. That, in many ways, is the difference between a Journeyman and a Master.
  • If some part of the clean-up could fail then should provide a Close method that will throw exceptions so the caller can handle them.
  • If you are following the dispose pattern (and you should be if the type directly contains some unmanaged resource) then the Dispose(bool) may be called from the finaliser, throwing from a finaliser is a bad idea and will block other objects from being finalised.

My view: exceptions escaping from Dispose should only be those, as in the guideline, that as sufficiently catastrophic that no further reliable function is possible from the current process.


I would argue that swallowing is the lesser of the two evils in this scenario, as it is better to raise the original Exception - caveat: unless, perhaps the failure to cleanly dispose is itself pretty darned critical (perhaps if a TransactionScope couldn't dispose, since that could indicate a rollback failure).

See here for more thoughts on this - including a wrapper/extension method idea:

using(var foo = GetDodgyDisposableObject().Wrap()) {
   foo.BaseObject.SomeMethod();
   foo.BaseObject.SomeOtherMethod(); // etc
} // now exits properly even if Dispose() throws

Of course, you could also do some oddity where you re-throw a composite exception with both the original and second (Dispose()) exception - but think: you could have multiple using blocks... it would quickly become unmanageable. In reality, the original exception is the interesting one.


Dispose should be designed to do its purpose, disposing the object. This task is safe and does not throw exceptions most of the time. If you see yourself throwing exceptions from Dispose, you should probably think twice to see if you are doing too much stuff in it. Beside that, I think Dispose should be treated like all other methods: handle if you can do something with it, let it bubble if you can't.

EDIT: For the specified example, I would write the code so that my code does not cause an exception, but clearing the TcpClient up might cause an exception, which should be valid to propagate in my opinion (or to handle and rethrow as a more generic exception, just like any method):

public void Dispose() { 
   if (tcpClient != null)
     tcpClient.Close();
}

However, just like any method, if you know tcpClient.Close() might throw an exception that should be ignored (doesn't matter) or should be represented by another exception object, you might want to catch it.


Releasing resources should be a "safe" operation - after all how can I recover from not being able to release a resource? so throwing an exception from Dispose just doesn't make sense.

However, if I discover inside Dispose that program state is corrupted it's better to throw the exception then to swallow it, its better to crush now then to continue running and produce incorrect results.


It's too bad Microsoft didn't provide an Exception parameter to Dispose, with the intention that it be wrapped as an InnerException in case disposal itself throws an exception. To be sure, effective use of such a parameter would require use of an exception-filter block, which C# doesn't support, but perhaps the existence of such a parameter could have motivated the C# designers into providing such a feature? One nice variation I'd like to see would be the addition of an Exception "parameter" to a Finally block, e.g.

  finally Exception ex: // In C#
  Finally Ex as Exception  ' In VB

which would behave like a normal Finally block except that 'ex' would be null/Nothing if the 'Try' ran to completion, or would hold the thrown exception if it did not. Too bad there's no way to make existing code use such a feature.