Should a .Net/C# object call Dispose() on itself?

Below is some sample code written by a colleague. This seems obviously wrong to me but I wanted to check. Should an object call its own Dispose() method from within one of its own methods? It seems to me that only the owner/creator of the object should call Dispose() when it's done with the object and not the object itself.

It's an .asmx web method that calls Dispose() on itself when it's done. (The fact that it's a web method is probably incidental to the question in general.) In our code base we sometimes instantiate web service classes within methods of other web services and then call methods on them. If my code does that to call this method, the object is toast when the method returns and I can't really use the object any more.

[WebMethod]
public string MyWebMethod()
{
    try
    {
        return doSomething();
    }
    catch(Exception exception)
    {
        return string.Empty;
    }
    finally
    {
        Dispose(true);
    }
}

UPDATE: Found a few links that are related:

Do I need to dispose a web service reference in ASP.NET?

Dispose a Web Service Proxy class?

Answers


For sure it's not a good prartice. The caller should decide when he is finished using the IDisposable object, not an object itself.


if I ever see that in one of my projects, I would ask why and I'm 99.9999% sure that i would remove it anyway

for me this is a kind of red flag / code smells


There are very few valid reasons to perform a "Self Disposing" action. Threading is the one I use more than not.

I have several applications that "fire and forget" threads. Using this methodology allow the object to self dispose.

This helps keep the environment clean without a threading manager process.


There are no technical restrictions on what a Dispose method is allowed to do. The only thing special about it is that Dispose gets called in certain constructs (foreach, using). Because of that, Dispose might reasonably be used to flag an object as no-longer-useable, especially if the call is idempotent.

I would not use it for this purpose however, because of the accepted semantics of Dispose. If I wanted to mark an object as no-longer-useable from within the class itself, then I would create a MarkUnuseable() method that could be called by Dispose or any other place.

By restricting the calls to Dispose to the commonly accepted patterns, you buy the ability to make changes to the Dispose methods in all of your classes with confidence that you will not unexpectedly break any code that deviates from the common pattern.


Just remove it, but take care to dispose it in all object that call it.


Technically yes, if that "method" is the finaliser and you are implementing the Finalise and IDisposable pattern as specified by Microsoft.


While a .Net object would not normally call Dispose on itself, there are times when code running within an object may be the last thing that expects to be using it. As a simple example, if a Dispose method can handle the cleanup of a partially-constructed object, it may be useful to have a constructor coded something like the following:

Sub New()
  Dim OK As Boolean = False
  Try
    ... do Stuff
    OK = True
  Finally
    If Not OK Then Me.Dispose
  End Try
End Sub

If the constructor is going to throw an exception without returning, then the partially-constructed object, which is slated for abandonment, will be the only thing that will ever have the information and impetus to do the necessary cleanup. If it doesn't take care of ensuring a timely Dispose, nothing else will.

With regard to your particular piece of code, the pattern is somewhat unusual, but it looks somewhat like the way a socket can get passed from one thread to another. There's a call which returns an array of bytes and invalidates a Socket; that array of bytes can be used in another thread to create a new Socket instance which takes over the communications stream established by the other Socket. Note that the data regarding the open socket is effectively an unmanaged resource, but it can't very well be wrapped in an object with a finalizer because it's often going to be handed off to something the garbage-collector can't see.


No! It is unexpected behavior and breaks the best practices guidelines. Never do anything that is unexpeceted. Your object should only do what is needed to maintain it's state while protecting the integrity of the object for the caller. The caller will decide when it's done (or the GC if nothing else).


Need Your Help

ASP.NET MVC 3 WebGrid paging issue

c# asp.net-mvc asp.net-mvc-3 webgrid

My data access layer returns collection with rows for single page and total number of rows.