Introducing SPWeb.GetParentWeb()

March 19, 2009 — Keith Dahlby

Official Microsoft guidance is to never explicitly Dispose() SPWeb.ParentWeb. I generally agree with this advice, given that my Rule #1 of SharePoint disposal is that “Using a disposed object can cause more problems than failing to dispose.” To understand why, I’ll borrow my explanation from SPDevWiki:

This property will allocate an SPWeb object the first time it is called. The caveat is that once it is disposed, any reference to the property will return the disposed object. If an SPWeb is not owned by the developer, its ParentWeb should be considered not owned as well. For example, there could be a problem if two components both depend on SPContext.Current.Web.ParentWeb and one calls Dispose() before the other is done with it.

However, this can result in memory pressure in cases involving enumeration or where the parent SPSite has a long lifetime. For example:

SPSite contextSite = SPContext.Current.Site;
foreach(SPWeb web in contextSite.AllWebs.AsSafeEnumerable())
{
    SPWeb webParent = web.ParentWeb; // Internal OpenWeb()
    // Do something with web and webParent
}

The web references are disposed by my safe iterator, but every webParent will remain open until the context SPSite is disposed. Not that I would recommend using code like this (in fact I would strongly urge against it), but you can never say never.

To that end, I propose a simple extension method whose contract is clear: always dispose me! We can still follow MS guidance regarding SPWeb.ParentWeb, but have convenient access to a developer-owned parent SPWeb as well:

[SPDisposeCheckIgnore(SPDisposeCheckID.SPDisposeCheckID_120, "By Design")]
public static SPWeb GetParentWeb(this SPWeb web)
{
    if(web == null)
        throw new ArgumentNullException("web");
    return web.Site.OpenWeb(web.ParentWebId);
}

And our “Best Practice” memory pressure can be revised slightly to achieve much better memory use:

SPSite contextSite = SPContext.Current.Site;
foreach(SPWeb web in contextSite.AllWebs.AsSafeEnumerable())
{
    using(SPWeb webParent = web.GetParentWeb())
    {
        // Do something with web and webParent
    }
}

Trivial? Obvious? Perhaps. But often the most useful code is.

Update: Included appropriate SPDisposeCheckIgnore attribute for “leaked” SPWeb from OpenWeb(); we know what we’re doing. That said, you could certainly implement higher-order functions to invoke an action or selector against our imitation ParentWeb without returning it—I’ll leave those as an exercise for the reader.

Advertisement
Posted in Object Model, SharePoint. Tags: , , , . 4 Comments »

The New Definitive SPSite/SPWeb Disposal Article

December 6, 2008 — Keith Dahlby

As I’ve posted before, properly cleaning up your SPSite and SPWeb objects is an important yet confusing consideration in writing good SharePoint object model code. There are some decent articles out there already, but Stefan Goßner just posted the best guidance – by far – I’ve seen on the subject: Disposing SPWeb and SPSite objects. Seriously, go read it.

In my view, Stefan hit on four key points that bear repeating:

1. Ensure that you only dispose SPSite and SPWeb objects that your code owns.

Examples that you don’t own, most of which I have seen incorrectly disposed in various examples online:

  • SPContext.Current.Web
  • SPContext.Current.Site
  • SPContext.Current.Site.RootWeb
  • SPFeatureReceiverProperties.Feature.Parent for a Site- or Web-scoped feature receiver
  • SPWebEventProperties.Web
  • SPListEventProperties.Web
  • SPListEventProperties.List.Web
  • SPItemEventProperties.ListItem.Web
  • UnsecuredLayoutsPage.Web (and LayoutsPageBase.Web)
  • SPWebProvisioningProperties.Web
  • SPControl.GetContextWeb()
  • SPControl.GetContextSite()

In general, there are three common cases where you do own the object:

  • When you new up your own SPSite object:
    SPSite site = new SPSite(url);
  • When you explicitly call SPSite.OpenWeb():
    SPWeb web = site.OpenWeb(url);
  • When you iterate through SPSite.AllWebs – this is an extremely expensive operation, so avoid if possible:
    foreach(SPWeb web in site.AllWebs) {
    // …
    web.Dispose();
    }

If you think of any other examples I should include in these lists, leave a comment and I’ll update the post.

2. An SPWeb or SPSite object you own should be disposed as soon as it is no longer needed.

Stefan’s post has a great explanation, which he neatly summarizes:

You should dispose a SPWeb or SPSite object after the last access to a child object of this object. Be aware that a sub site (SPWeb) is not a child object. But (e.g.) a list or a folder or list item is a child object for this scenario.

3. Using a disposed SPWeb or SPSite object (like might happen if you Dispose something that doesn’t belong to you) can cause exceptions.

While it’s great to Dispose properly, it’s dangerous to Dispose improperly: when in doubt, in my opinion, err on the side of caution and let SharePoint clean up the mess. For webs, that happens when the parent site is disposed. For sites, that happens when the thread ends. If you do have memory problems, check out Stefan’s post on Troubleshooting SPSite/SPWeb leaks in WSS v3 and MOSS 2007.

4. SPSite and SPWeb objects should be disposed in the same method they get allocated.

And as a corollary to this, I agree with Chris O’Brien‘s conclusion, almost as an afterthought, in a recent post:

However, if the caller has to provide the IDisposable objects, it can then be responsible for calling Dispose() because it knows when they are no longer needed. Most often the caller will simply be passing a reference to SPContext.Current.Web, but if the caller happens to be a Feature receiver then SPFeatureProperties.ListItem.Parent.Web would be passed, or if no existing SPWeb object was available to the caller (e.g. console app) a new SPWeb object would have to be instantiated and passed. Disposals then become much simpler because they can always happen in the same place as any instantiations.

If the logic that utilizes the SPSite/SPWeb is encapsulated within a function that receives it as an argument, there is no risk of leaks. Most simply, there is little risk when the code looks like this:

using (SPWeb web = SPContext.Current.Site.OpenWeb()) {
  DoSomethingWith(web);
}

Now, some developers like to encapsulate the SPSite/SPWeb retrieval logic in dedicated methods. A safe approach is to leverage delegates. Unfortunately, variations of an unsafe alternative are much more common (in this case, copied from an otherwise brilliant MVP’s blog):

public static SPWeb ReturnSPWeb(string url) {
  using (var site = new SPSite(url)) {
    using (SPWeb web = site.OpenWeb()) {
      return web;
    }
  }
}

When you return from within a using block, the compiler will insert a Dispose on the used object before returning. Open your assembly in ildasm if you don’t believe me. So the above code is effectively equivalent to the following:

public static SPWeb ReturnSPWeb(string url) {
  var site = new SPSite(url);
  SPWeb web = site.OpenWeb();
  web.Dispose();
  site.Dispose();

  return web;
}

Oops! Before they even got a chance to do anything, the objects were cleaned up, and more importantly any code that depends on the returned SPWeb is completely unsafe! The same would apply for child objects (SPList, etc) returned from within a using block around the parent object. Even if the code works most of the time, the bugs it could introduce would be nearly impossible to track down.

Thanks again to Stefan for the excellent post!

Last updated 12/8/2008

Posted in Object Model, SharePoint. Tags: , , , . 12 Comments »

Is SPList.ParentWeb a Leak?

August 10, 2008 — Keith Dahlby

As previously discussed, doing the “right thing” with our SPSite and SPWeb references’ disposability is way harder than it should be. And even worse is the fact that the experts don’t seem to agree how that should be done. Now I’m definitely not an expert, and I’m not even sure who’s moderating the debate, but I’m intrigued so I guess I’ll invite myself anyway.

First, the code in question, from a useful STSADM extension by Gary Lapointe:

  lvw = new ListViewWebPart();
  SPList list = Utilities.GetListFromViewUrl(listUrl);
  if (list == null)
    throw new ArgumentException("List not found.");

  lvw.ListName = list.ID.ToString("B").ToUpperInvariant();
  lvw.TitleUrl = list.DefaultViewUrl;
  lvw.WebId = list.ParentWeb.ID;

The question: Does list.ParentWeb need to be disposed? My first instinct was that it does, based on this quote from Roger Lamb’s dispose patterns:

When using the SPList.BreakRoleInheritance() method a internal call to ParentWeb property is called and must be disposed by the caller.

But upon further consideration, is that even true? Well, let’s reflect SPList.ParentWeb to see what we’re dealing with:

    if (!SPUtility.StsCompareStrings(this.m_Lists.Web.ServerRelativeUrl, this.ParentWebUrl)) {
      if (this.m_parentWeb == null) {
        this.m_parentWeb = this.m_Lists.Web.Site.OpenWeb(this.ParentWebUrl);
      }
    }
    else {
      return this.m_Lists.Web;
    }
    return this.m_parentWeb;

Or rewritten for clarity:

    if (SPUtility.StsCompareStrings(this.m_Lists.Web.ServerRelativeUrl, this.ParentWebUrl))
        return this.m_Lists.Web;

    if (this.m_parentWeb == null)
        this.m_parentWeb = this.m_Lists.Web.Site.OpenWeb(this.ParentWebUrl);
    return this.m_parentWeb;

So if the list’s parent collection matches its ParentWebUrl, it uses the collection’s Web; if not it returns m_parentWeb, which Reflector verifies is only set by the preceding call to Site.OpenWeb(). As far as I can tell, every call to OpenWeb requires a matching call to Dispose, and as SPList isn’t IDisposable that responsibility would seem to fall to the developer. So we should Dispose, right? Well, maybe… We can’t forget about the first if: OpenWeb is only called if the list isn’t in its collection’s web. (Bonus question: how could that happen?)

So it seems the most common scenario is that SPList.ParentWeb will just defer to SPListCollection.Web, which returns SPListCollection.m_web. And m_web is only set by an internal SPListCollection constructor, used by SPWeb.Lists:

    if (this.m_Lists == null)
      this.m_Lists = new SPListCollection(this);
    return this.m_Lists;

So with all that in mind, consider the following code:

    SPWeb web = SPContext.Current.Web;
    SPList list = web.Lists["ListName"];
    list.BreakRoleInheritance();
    list.ParentWeb.Dispose(); // Best practice?

BreakRoleInheritance does indeed have an internal reference to ParentWeb (hidden in SPList.SecurableObjectImpl), but in this case ParentWeb will be web, which is our context web and should not be disposed!

So back to Gary’s example. Before we can decide how to handle list.ParentWeb, we should check out the helper that created list for us:

    internal static SPList GetListFromViewUrl(string url) {
      using (SPSite site = new SPSite(url))
      using (SPWeb web = site.OpenWeb()) {
        return GetListFromViewUrl(web, url);
      }
    }

Since the SPWeb was opened based on the list URL, it should be safe to assume that ParentWeb won’t need to call OpenWeb again and will instead use Lists.Web, which will be a reference to the original web…which is disposed (by using) in the helper. So list.ParentWeb indeed doesn’t need to be disposed because it’s already been disposed. In practice we can probably get away with it (in this case), but there are no guarantees that a disposed SPWeb will be in a consistent state when we get around to using it.

Though we’ve answered the original question of disposal, we should really eliminate the post-disposal reference. My preference is to refactor the ListViewWebPart code into helpers modeled after Gary’s existing GetListFromViewUrl methods:

    internal static ListViewWebPart GetListViewWebPartFromViewUrl(string url) {
      using (SPSite site = new SPSite(url))
      using (SPWeb web = site.OpenWeb()) {
        return GetListViewWebPartFromViewUrl(web, url);
      }
    }

    internal static ListViewWebPart GetListViewWebPartFromViewUrl(SPWeb web, string url) {
      ListViewWebPart lvw;
      lvw = new ListViewWebPart();
      SPList list = GetListFromViewUrl(web, url);
      if (list == null)
        throw new ArgumentException("List not found.");

      lvw.ListName = list.ID.ToString("B").ToUpperInvariant();
      lvw.TitleUrl = list.DefaultViewUrl;
      lvw.WebId = web.ID;
      return lvw;
    }

Which greatly simplifies the original code and eliminates the ParentWeb risk:

  lvw = Utilities.GetListViewWebPartFromViewUrl(listUrl);

Conclusions

So back again to the original question: Does SPList.ParentWeb need to be disposed? The answer seems to be a qualified “probably not”. But even if we’re not leaking an SPWeb reference, more concerning is the potential for ParentWeb to return an object that has already been disposed. If nothing else, be careful with ParentWeb on lists returned from helpers that might have cleaned up prematurely.

Posted in Object Model, SharePoint. Tags: , , , , . 2 Comments »