How much work in the Dispose method?

Question

How much work should be done in a Dispose method? In constructors I've always taken the stance that you should only do what is absolutely necessary to instantiate the object. This being the case I've also always taken the approach that you should ONLY be cleaning up open resources when disposing. Closing files, freeing memory, disposing of child disposable object, etc. You shouldn't be doing lengthy processes like touching files, accessing databases and such in the Dispose method.

Am I wrong? Are those action's OK as long as you are handling any possible exceptions so they don't bubble out of the method? I just don't think doing a lot in Dispose is a good idea. I would like to know what the community thinks.

Answer 1

Am I wrong?

No, you are right. In general the Dispose method is used to clean the unmanaged resources that your class might have allocated.

But that's difficult to generalize. There are cases where the Dispose method is simply used to ensure that some operation executes. For example in ASP.NET MVC there's the Html.BeginForm helper which is used like that:

using (Html.BeginForm())
{

}

and all that the Dispose method does is render a closing </form> tag. So as you can see people could be creative with the pattern and it is very difficult to draw conclusions without a specific scenario.

But in the most common situations it's used to release unmanaged resources.

Answer 2

"It depends". What kind of database/file access are we talking about? Say for example that your disposable object is some sort of logger and you use it in the following pattern

using(Logger logger = new Logger())
{
    foo.PerformTask();
}

I think it would be perfectly acceptable for logger to write out "Log started" in the constructor "Log Completed" in Dispose.

Answer 3

How much work should be done in a Dispose method?

This depends, are you implementing the IDispose interface just for the convenience of a 'using' statement, or are you implementing the full IDisposable pattern? In the later case of a full disposable pattern it is still acceptable to perform more complex actions provided that you're 'disposing' parameter is true (i.e. you are not in GC).

When you are defining a finalizer that calls the Dispose method there really is not too much to be concerned about. Similar uses/abuses of the IDisposable interface mentioned already be others (i.e. using (Html.BeginForm())) are capable of performing any action. Often this can greatly reduce code complexity and prevent coders from accidentally forgetting to perform some closing action. One down (or up) side to this is that code executes a little differently inside a finally block.

In constructors I've always taken the stance that you should only do what is absolutely necessary to instantiate the object.

Objects, IMHO, should be valid post construction. So if you have a lot of work to do to construct something so be it. Don't think of the workload involved, think of the consumer of you're object and it's usability. Post-construction Initialize() methods suck ;)

This being the case I've also always taken the approach that you should ONLY be cleaning up open resources when disposing. Closing files, freeing memory, disposing of child disposable object, etc. You shouldn't be doing lengthy processes like touching files, accessing databases and such in the Dispose method.

Actually let's break this down a bit...

Disposing from the GC call to the Finalizer

When you implement the IDisposable pattern (not the interface, the pattern, finalizer and all) you are essentially saying that your object has an unmanaged resource that nobody else knows about. That means you have PInvoked a call to Win32's CreateFile, or maybe you called Marshal.AllocHGlobal or something like that. Essentially you likely have an IntPtr instance member you need to do something with to prevent a memory leak. These are the ONLY types of things that should be done when the disposing parameter is false (i.e. called from the finalizer on the GC thread).

Generally you DO NOT call the Dispose method on children. You should not expect any child object to be valid. Simply touching a member of the child object can accidentally 'revive' or resurrect it.

So when you are writing code that executes in a Dispose method called from the Finalizer you have to be careful. You are executing on the GC thread while the rest of your application waits for you. You should perform as few operations as possible to release the unmanaged memory/resource and quit. Never throw an exception and if you are calling an API that may throw you should catch any exception raised. Propagating exceptions back to the GC will prematurely abort the finalizer thread and the remaining objects to be finalized will not have a chance to clean up.

Disposing from the IDisposable.Dispose() method

As I’ve already said, using the Dispose method is safe enough and can safely accommodate any amount of code/process. This is where you would free unmanaged resources, call the dispose method of child objects, flush and close files, etc. Most of the Dispose methods I’ve written do not have an associated Finalizer and therefore do not follow the IDisposable pattern, yet they implement IDisposable just for the convenience of the using statement.

Am I wrong? Are those action's OK as long as you are handling any possible exceptions so they don't bubble out of the method? I just don't think doing a lot in Dispose is a good idea. I would like to know what the community thinks.

You are absolutely right when the dispose method in question is used from a finalizer. You’re assertions about what you should and should not do in a Dispose method should actually be reworded to apply to anything called by a Finalizer. The fact that this is generally done in a method called Dispose is a matter of convention, the IDisposable pattern, but these issues could easily exist in other methods used by the Finalizer.

Answer 4

If an object does something to the state of some outside entities in a way which makes them more useful to that but less useful to everyone else, the Dispose method of the object should do whatever is necessary to restore outside those entities to more generally-useful state. If one wishes to avoid having an object do too much work in Dispose, one should design the object so as never leave any outside entities in a state which would be onerous to clean up.

BTW, Microsoft likes to use the term "unmanaged resources", and gives examples, but never really offers a good definition. I would suggest that an object holds an "unmanaged resource" if an outside entity is altering its behavior on behalf of that object, in a fashion which is detrimental to other objects or entities, and if that outside entity will continue to alter its behavior until the object stops it from doing so.

Answer 5

You should lean toward the conclusion you have already come to. However there are situations where you need to ensure that services are stopped and that could include things like messages being logged for the service shutdown, or saving the current runtime state to data store. This type of disposal usually only applies to things that have a lifestyle that is application scope, meaning they exist the whole time the application is running. So there are situations outside of the expected norm. As is the case with every rule you should follow when writing code.