Solutionizing .NET

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.