To Dispose or not to Dispose? That is the question.

Having just read a new book about SharePoint 2010, I started thinking about the code samples used, and the best practices discussed in the book. It got me thinking about the to Dispose or not to Dispose question, when dealing with SPSite and SPWeb objects.

The book mentions a disposable pattern used for SPSite and SPWeb and a SharePoint best practice:

Disposable pattern: As SPSite and SPWeb implement the IDisposable interface, you should Dispose these objects, because they consume a lager amount of unmanaged memory.

Best practice: When using SPContext.Current.Site, SPContext.Site, SPContext.Current.Web or SPContext.Web these objects should not be disposed in user code.

So you should dispose SPSite and SPWeb, and also not dispose the SPContext ones, but they explain this by saying 'it is a SharePoint best practice'. That's not really an explanation. Therefore I will try to explain how I think about this. Also a salient detail, in subsequent code samples they sometimes use:

using(SPWeb web = SPContext.Current.Web) //Bad

Of course this is a mistake in the sample source, but you can see that such a mistake is easily made. We as developers should be aware of that.

So why shouldn't you dispose SPContext's Current Site and Web objects?
If you know something about Object Orientation, you probably know that Responsibility is a important concept in OO. In my opinion this concept is the answer for when to dispose, or not. It is about the object that created SPSite or SPWeb, is the owner of the object, or even better is the Information Expert. As your user code did not create the Site or Web object, is not the owner and hasn't the right information when to dispose it, it is not responsible for disposing that object.

Why in these cases disposing is a bad thing? Think about subsequent code in the same request, that could use the SPContext to program against the Site or Web. This code could get in trouble when it uses functionality of an already disposed object. The SharePoint system that created the Current SPContext is responsible for the disposing, it 'knows' when it is save to dispose the object.

Ok, I hope this explains things. But then I saw other code samples in the book that made me think more about this. For example something like this:

using(SPSite mySite = new SPSite("http://localhost"))
{
    SPSite iteratedSite = null;
    foreach(iteratedSite in mySite.WebApplication.Sites)
    {
        try
        {
            //do something with iteratedSite
        }
        finally
        {
            if(iteratedSite != null)
                iteratedSite.Dispose();
        }
    }
}

Here every iteratedSite gets disposed in the finally statement. But what about the creator/owner object who is responsible for disposing, isn't that mySite.WebApplication that contains the collection Sites? Well, lets see: mySite.WebApplication is the creator of the Site objects in the collection, and it is the owner, but our code sample is the information expert on when to dispose the Site object in the collection. We don't need these 'expensive' objects anymore when we are finished with each loop. We are sure of that because we created and own the mySite object, and this object owns the SPWebApplication object, which owns the Sites collection containing the site.

Ok, but then what if mySite is a reference to the SPContext.Current.Site? Other code could then also loop through the SPContext.Current.Site.WebApplication.Sites and encounter disposed objects. So then our user code isn't the information expert. As we don't know, or are not supposed to know, the inner workings of the SharePoint objects we should leave the responsibility to the SPContext.Current.Site object. Does this object dispose the WebApplication.Sites? I really don't know, SPWebApplication does not implement IDisposable, but the SPSite could take care of it. But you can be sure that it at least doesn't dispose these objects in a timely matter. Maybe this is the time you would want to use some silly looking code, that I also encountered in the book:

using(SPSite mySite = new SPSite(SPContext.Current.Site.Url))

This ensures that your code is the owner of mySite, and therefore is also the information expert. You do however create an extra SPSite object containing the 'expensive' resource, but it will dispose everything as soon as possible. To me this seems the best solution. By the way, I think it's silly looking because: Why create a Site object from a Site object? But now you know a reason why you would want to do this.

Ok, now I will briefly discuss some other code samples I encountered. Some samples contained:

using(SPWeb myWeb = mySite.OpenWeb()) //Good

By calling a method that returns an object, your code becomes the owner of that object, and is responsible for the disposal of the object. Even if mySite is a reference to SPContext.Current.Site. Yet an other example that was in the book:

using(SPWeb myWeb = mySite.RootWeb) //Bad (in my opinion)

Because this is a property of SPSite, the SPSite object is the owner and is responsible for disposing the object, and not your user code. This isn't different for an SPSite object created by you, or the SPContext.Current.Site object. In a way you could argue that it's good if you created the mySite object yourself. But you are not aware of the internal usage of this SPWeb object exposed by the property RootWeb by SPSite. You can release this resource some what quicker in these cases, but theoretically this is not the right way of dealing with the object.

As you can see, you should be aware of the differences in dealing with Properties, Methods and Collections of SPSite or SPWeb when using your own SPSite/SPWeb objects or using those in SPContext.Current. I hope this article helped you gain some extra insights on the question: To Dispose or not to Dispose?

Also take a look at:
Best Practices: Using Disposable Windows SharePoint Services Objects

That page contains an amazing amount of info on the Disposing of objects in SharePoint. I think it is very interesting especially when working with collections of SPSite or SPWeb objects. The article doesn't have examples on using these collection on a SPContext.Current context. You would be better off creating your own SPSite and SPWeb when using these collections, as you have seen in this article.

Added 10/17/2010:

Thinking about what I wrote, I thought about the try-finally construction used in the example when iterating through a collection. I really find the try-finally construction rather ugly and prefer using the using construction. So what about this:

using(SPSite mySite = new SPSite("http://localhost"))
{
    SPSite iteratedSite = null;
    foreach(iteratedSite in mySite.WebApplication.Sites)
    {
        using(SPSite iteratred = iteratedSite)
        {
            //do something with iterated or iteratedSite (same object)
        }
    }
}

As the using statement is practically the same as try-finally, they should be interchangeable. As you can see in this example, they are. This of course works because objects are reference types, so iterated and iteratedSite site refer to the same object. It doesn't mater if you use iterated or iteratedSite in the using block. Yes, it probably looks a bit strange to some people, but I rather like this, and it's less code.

One other thing I would like to mention is that I kind of cheated a little bit in dealing differently with properties and collections. Yes, the collections are properties... I deal with collections differently because you have to be pragmatic. As you can see in the examples in the Best Practices article, this is the way things work in SharePoint, so you can assume that these collections are not used internally by the objects that have them as a property.

Added 10/20/2010:

Yet an other addition to this text. As I am now reading an other SharePoint 2010 book, I came across some code that I thought would be a nice example here. This time it is sample code of a FeatureReceiver:

public override void FeatureActivated(SPFeatureReceiverProperties properties)
{
    //Bad (in my opinion, this is what I want to discuss)
    using(SPSite site = (SPSite)properties.Feature.Parent) 
    {
        using(SPWeb web = site.RootWeb) //Bad (in my opinion, as already discussed)
        {
            //some code in the sample (creating content type)
        }
    }
}

Do you already know what I want to say? Well, as you are using a property of properties.Feature, you are (in my opinion) not supposed to Dispose this. Your code didn't create the object (only did a cast), you didn't get ownership and you don't know if there is other code that uses this Parent property (your are not the information expert).

As you could also have some code in this method that doesn't use properties.Feature.Parent, so the SharePoint system is the one that is responsible for disposing this Parent property. For all you know, this property could well be the same object reference as SPContext.Current.Site, which you are not supposed to Dispose yourself.

As you can see, it's not only about disposing, but also about not disposing if your code is not responsible for it. So the question to dispose or not to dispose is quite a hard one. But if you get the way of thinking about this, the question might not be that hard after all.

Added 11/27/2010:

Recently I have been asked to solve some issues with a SharePoint 2007 application. One of the first things I did was running SPDisposeCheck on the custom code because I had noticed that not all SPSite and SPWeb objects where disposed. It found some issues, but I noticed that it didn't find a specific issue that I saw in the code. I have created some sample code and did a SPDisposeCheck using version 1.3.1. Here is the sample source and SPDisposeCheck output:

public class TestDispose : SPItemEventReceiver
{
    public override void ItemAdding(SPItemEventProperties properties)
    {
        Guid webId = properties.OpenWeb().ID; // SPDisposeCheck does not find this
        //SPWeb web = properties.OpenWeb();
        //Guid webId = web.ID;
    }
}

Processing Method Traces...
Total Found: 0
----------------------------------------------------------
Modules Checked: 1
----------------------------------------------------------

Both the properties.OpenWeb().ID and the properties.OpenWeb() didn't show up as an issue. So don't rely on SPDisposeCheck only, and know that the best way of preventing problems is to understand what you are doing.

I also found a nice blog post that may be of interest to you:
SharePoint 2007/2010 “Do Not Dispose Guidance” + SPDisposeCheck

I noticed the author mentioning the following methods where you should not dispose the returned object: SPControl.GetContextWeb(..) and SPControl.GetContextSite(..). As I generalized a difference between Properties, Methods and Collections I find it important to notice that this is just a generalization. Don't stop thinking for your self. You should understand that these methods will return a reference to the SPContext Site and Web objects and therefore should not be disposed.