SharePoint Disposal Wish List

I was almost off my SharePoint disposal kick when Jeremy Jameson had to pull me back in with this post. Resisting the temptation to rehash some stuff I’ve covered before, I thought it might be therapeutic to spend some time discussing what the dispose story should look like. I’m sure the API for WSS 4.0 is more or less locked at this point, but it still might be useful to frame how I think about ownership of these blasted IDisposables.

In my SharePoint fantasy world, the dispose story can be explained in four bullets:

  1. Use of a disposed object throws an exception; failures to dispose properly are logged.
  2. IDisposable references retrieved from an instance property are owned by the instance.
  3. IDisposable references retrieved from a constructor, method or collection indexer are owned by the developer and should always be disposed.
  4. IDisposable references from an enumerator are owned and disposed by the enumerator.

Let’s examine these a bit more closely.

1. Disposed = Invalid = Use is Exceptional; Discoverability = Good

The leaked object logging is already pretty good, but that’s more than made up for by the epic failure to actually close objects that have been closed. Fixing discoverability would make all other points a matter of convenience.

2. Instance Properties

An instance property suggests “mine”. My context’s Site, my site’s RootWeb, my feature’s Parent, etc. If something is “mine,” then I should be responsible for it. This behavior is already consistent with three exceptions:

SPWeb.ParentWeb

This property is tricky because the instance is shared for all consumers of the SPWeb (think SPContext.Web), but will not be cleaned up if the SPWeb is disposed and has no mechanism to notify the child that it has been disposed to null out the reference to it. So what to do? Well in accordance with my fantasy guidelines, there are two options, either of which would resolve the ambiguity:

  1. Leave ParentWeb as a property and require that the SPWeb dispose its parent as part of the clean-up process.
  2. Provide SPWeb.GetParentWeb() instead, returning a new SPWeb on each call that is owned by the developer.

Since GetParentWeb() can be implemented as an extension method, I guess I would prefer option 1.

SPList.ParentWeb

This property is almost fine, but for some reason has an edge case returns a new SPWeb (details here). It should be fine as a property, but that edge case needs to disappear. Why can’t it just return SPList.Lists.Web?

MOSS UserProfile.PersonalSite

This property returns a new SPSite on each call, so ideally it would be a method instead, perhaps GetPersonalSite. That seems much easier than having UserProfile implement IDisposable to facilitate cleanup of shared instance.

3. Constructors & Methods & Indexers, Oh My!

If properties are “mine”, then everything else is “yours”. Again, the vast majority of the API already fits this behavior. The few discrepancies:

SPControl.GetContextSite() & SPControl.GetContextWeb()

I believe these are the only methods that return IDisposables that SharePoint owns. MSDN clearly indicates that behavior, but for consistency the preferred usage (consistent with our fantasy guidelines) would be to use an SPContext’s Site and Web properties instead.

MOSS PublishingWeb

I’ve already discussed the semi-disposability of PublishingWeb, which just needs a few tweaks to fit nicely in my fantasy model:

  • Implement IDisposable! Close() is already implemented, just need the interface to take advantage of our languages’ using constructs.
  • I believe ParentPublishingWeb would be fixed if SPWeb.ParentWeb were disposed with the child. Alternatively, change to GetParentPublishingWeb and ensure that the returned PublishingWeb will close the internal SPWeb on dispose.
  • PublishingWebCollection.Add() is fine, but the indexer works in such a way that an internal SPWeb is created that is not cleaned up by PublishingWeb.Close(). I would consider this a bug in the current code base, which would naturally be fixed in my fantasy.

4. Enumerating Collections

This isn’t quite as simple as it would seem. When dealing with collections and enumerators, there are essentially two classes of operation:

  1. Enumerate over all or part of the collection and perform an operation. (LINQ Select, GroupBy, OrderBy)
  2. Enumerate over the collection to pick an object from the collection to return. (LINQ First, Last, ElementAt)

In my experience, the vast majority fall into the first category, including most of LINQ. These operations shouldn’t need to know that the objects being enumerated are IDisposable; it’s up to the enumerator to behave appropriately. Rather than sacrifice these many useful operations, I suggest that the enumerator should include proper disposal of the objects it creates—this is precisely what my AsSafeEnumerable() iterator does.

The second category can either be handled using for loops and indexers, which return objects that the caller must dispose, or through higher-order functions that allow dispose-safe operation on or selection from the desired element. But again, these seem to be the exception rather than the rule, and can be handled accordingly.

Wishful Thinking

The unfortunate reality is that most of these adjustments could never happen because breaking changes are generally frowned upon. But perhaps in framing our disposal thought process around some simple rules with documented exceptions, it may be easier to get things right without leaning on tools like SPDisposeCheck.

Posted in Object Model, SharePoint. Tags: , . Comments Off on SharePoint Disposal Wish List

Introducing SPWeb.GetParentWeb()

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.

SharePoint+PowerShell Leak Workarounds

Zach Rosenfield recently posted about an obscure memory leak issue when working with PowerShell and the SharePoint object model. You can read his post for details but in summary:

  1. PowerShell spawns a new thread for each pipeline (essentially any batch of code that runs together).
  2. SharePoint objects should not be used across multiple threads due to mechanics of the unmanaged heap.
  3. SharePoint objects used across multiple pipelines result in unmanaged heap leaks until PowerShell closes or you run out of memory.

This PowerShell behavior is easy enough to verify:

PS 1> function Get-ThreadId { [Threading.Thread]::CurrentThread.ManagedThreadId }
PS 2> set-alias gt Get-ThreadId
PS 3> gt
6
PS 4> gt
4

So we need to ensure that our SharePoint objects are allocated, used and disposed within the same thread. How can we do this? Zach offers two suggestions, but there are actually several options:

Single Line

PS 5> gt; gt; gt
10
10
10
PS 6> gt; `
>> gt; `
>> gt
>>
3
3
3

Script

PS 7> @"
>> gt
>> gt
>> "@ > gt.ps1
>>
PS 8> .\gt.ps1
8
8

Function

PS 9> function gt2 {
>> gt
>> gt
>> }
>>
PS 10> gt2
4
4

Inline Script Block

PS 11> &{
>> gt
>> gt
>> }
>>
7
7

My Using Function

PS 12> using Microsoft.SharePoint
PS 13> gt; using ($s = [Microsoft.SharePoint.SPSite] 'http://moss') {
>> gt
>> $s.Url
>> gt
>> }
>>
5
5
http://moss
5

Cmdlet + Wrapper Class

Gary Lapointe has a set of PowerShell cmdlets that use wrapper objects to hide SPSite/SPWeb objects unless you specifically ask for them.

PS 14> $spi = Get-SPSite-gl 'http://moss'
PS 15> gt; using ($s = $spi.SPBase) { $s.Url; gt }
8
http://moss
8

The severity of this issue depends on your environment and the kinds of scripts you’re running, but in memory-sensitive and production environments these are definitely some techniques to keep in mind.

Do Not Dispose SPFeatureReceiverProperties.Feature.Parent

I can’t find a dedicated post about it, so I thought I’d draw attention to this thread in the MSDN forums where Michael Washam [MSFT] replied…

Do not dispose properties.Feature.Parent..

However, if you use .ParentWeb or anything else the same rules apply..

Now, the rule for SPWeb.ParentWeb is that it should not be disposed if you don’t own the child SPWeb, which we don’t, but you get the idea. It would be great if SPDisposeCheck would catch this—Roger?

I only point it out specifically because feature receiver examples are often posted online and I see Parent disposed all the time.

Avoiding UserProfile.PersonalSite Leaks

Roger Lamb has posted another edge case in the pursuit of leak-free object model code: UserProfile.PersonalSite. To help follow his best practices, I offer another round of extension methods. First, for UserProfile:

public static void ProcessPersonalSite(this UserProfile profile, Action<SPSite> action)
{
    using (SPSite site = profile.PersonalSite)
    {
        action(site);
    }
}

public static T SelectFromPersonalSite<T>(this UserProfile profile, Func<SPSite, T> selector)
{
    using (SPSite site = profile.PersonalSite)
    {
        return selector(site);
    }
}

Usage:

void PersonalSiteNoLeak()
{
    // open a site collection
    using (SPSite siteCollection = new SPSite("http://moss"))
    {
        UserProfileManager profileManager = new UserProfileManager(ServerContext.GetContext(siteCollection));
        UserProfile profile = profileManager.GetUserProfile("domain\\username");
        profile.ProcessPersonalSite(personalSite =>
        {
            // Process personalSite
        });
    }
}

int CountContextPersonalSiteWebs()
{
    UserProfile myProfile = ProfileLoader.GetProfileLoader().GetUserProfile();
    return myProfile.SelectFromPersonalSite(personalSite =>
    {
        return personalSite.AllWebs.Count;
    });
}

I’ve also encapsulated the verification logic for accessing a MySite web part’s PersonalSite, with variations for projection and with/without exceptions:

public static void ProcessPersonalSite(this WebPart webpart, Action<SPSite> action)
{
    IPersonalPage personalPage = webpart.Page as IPersonalPage;
    if (personalPage == null)
        throw new SPException("Unable to access personal site. Invalid page.");
    if (personalPage.IsProfileError)
        throw new SPException("Unable to access personal site because of a profile error.");

    action(personalPage.PersonalSite);
}

public static T SelectFromPersonalSite<T>(this WebPart webpart, Func<SPSite, T> selector)
{
    IPersonalPage personalPage = webpart.Page as IPersonalPage;
    if (personalPage == null)
        throw new SPException("Unable to access personal site. Invalid page.");
    if (personalPage.IsProfileError)
        throw new SPException("Unable to access personal site because of a profile error.");

    return selector(personalPage.PersonalSite);
}

public static bool TryProcessPersonalSite(this WebPart webpart, Action<SPSite> action)
{
    IPersonalPage personalPage = webpart.Page as IPersonalPage;
    if (personalPage == null || personalPage.IsProfileError)
        return false;

    action(personalPage.PersonalSite);
    return true;
}

public static bool TrySelectFromPersonalSite<T>(this WebPart webpart, Func<SPSite, T> selector, out T value)
{
    value = default(T);
    IPersonalPage personalPage = webpart.Page as IPersonalPage;
    if (personalPage == null || personalPage.IsProfileError)
        return false;

    value = selector(personalPage.PersonalSite);
    return true;
}

Usage:

protected override void CreateChildControls()
{
    base.CreateChildControls();

    this.ProcessPersonalSite(personalSite =>
    {
        // Process personalSite
    });
}

As always, feedback is encouraged.

The Semi-Disposable PublishingWeb

As if handling SPSite and SPWeb isn’t tedious enough, MOSS’s PublishingWeb class throws another wrench into things. Its constructors are internal, so references to it always come from within the framework. I’m currently aware of five sources of these objects:

  1. PublishingWeb.GetPublishingWeb() wraps an existing SPWeb reference that should already have a disposal plan.
  2. PublishingWeb.GetVariation() creates an internal SPWeb that is disposed on Close().
  3. PublishingWeb.ParentPublishingWeb creates an internal SPWeb that is disposed on Close(), but also references its Web‘s ParentWeb, which may or may not need to be disposed.
  4. PublishingWebCollection creates an internal SPWebCollection from which it pulls SPWebs that are not disposed on Close() of the child PublishingWebs.
  5. PublishingWebCollection.Add() creates an internal SPWeb that is disposed on Close().

Internally, PublishingWeb is set up relatively well to handle the complexity of wrapping a Disposable object. It provides two constructors: one that accepts an SPWeb, which it stores internally but does not mark as “owned”; and one that accepts a Uri and SPSite, from which it opens an SPWeb (and SPSite if necessary) that are marked as “owned” to be cleaned up later. This job is performed by Close(), which is even kind enough to add a trace message (level Medium) if an attempt is made to close a non-owned web:

PublishingWeb.Close(): currentWeb is owned by caller, so doing nothing. May indicate SPWeb leak in calling code.

One could argue that PublishingWeb should really be IDisposable, calling Close() from Dispose() (like SPWeb does), but for now we can get by with try...finally.

The Good

The internal ownership mechanism serves its purpose wonderfully for GetPublishingWeb(), GetVariation() and PublishingWebCollection.Add(). The former just defers to the SPWeb-accepting constructor with no need to close, and the latter two use the Uri+SPSite constructor and can be closed without hesitation.

using(SPWeb web = site.OpenWeb())
{
  PublishingWeb varPubWeb = null
  PublishingWeb newPubWeb = null;
  try
  {
    PublishingWeb pubWeb = PublishingWeb.GetPublishingWeb(web);

    VariationLabel label = Variations.Current[0];
    varPubWeb = pubWeb.GetVariation(label);
    // Process varPubWeb

    PublishingWebCollection pubWebs = pubWeb.GetPublishingWebs();
    newPubWeb = pubWebs.Add("NewPubWeb");
  }
  finally
  {
    // pubWeb.Close() would generate a trace warning
    if(null != varPubWeb)
      varPubWeb.Close();
    if(null != newPubWeb)
      newPubWeb.Close();
  }
}

The Bad

Things get a bit trickier with ParentPublishingWeb for two reasons:

  1. We are subject to all the usual concerns when dealing with SPWeb.ParentWeb:

    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.

  2. Internally ParentPublishingWeb uses the “owned” constructor, so we need to call Close(). However, it gets the parameters for that constructor from its Web‘s ParentWeb property. If we’re in a situation where we own the original SPWeb, we also now own its ParentWeb and will need to Dispose() it.

In a case where we definitely own the web and its ParentWeb, we need to do something like this:

using(SPWeb web = site.OpenWeb())
{
  PublishingWeb parentPubWeb = null;
  try
  {
    PublishingWeb pubWeb = PublishingWeb.GetPublishingWeb(web);
    parentPubWeb = pubWeb.ParentPublishingWeb;
    // Process parentPubWeb
  }
  finally
  {
    if(null != parentPubWeb)
      parentPubWeb.Close();
    if(null != web.ParentWeb)
      web.ParentWeb.Dispose();
  }
}

The Ugly

Given how hard PublishingWeb tries to do the right thing, I’m surprised how little PublishingWebCollection does to help us. It too has an internal constructor, so we can only get a collection from PublishingWeb.GetPublishingWebs(), which passes in the current web’s GetSubwebsForCurrentUser(). This SPWebCollection is then used to build the PublishingWeb objects returned by indexing into or enumerating over the collection. However, unlike PublishingWebCollection.Add(), the indexers use GetPublishingWeb(SPWeb) and thus the “not my problem” constructor. So rather than rely on Close(), we need to manually call Dispose() on the wrapped Web:

using(SPWeb web = site.OpenWeb())
{
  PublishingWeb pubWeb = PublishingWeb.GetPublishingWeb(web);
  PublishingWebCollection pubWebs = pubWeb.GetPublishingWebs());
  foreach(PublishingWeb innerPubWeb in pubWebs)
  {
    try
    {
      // Process innerPubWeb
    }
    finally
    {
      // innerPubWeb.Close() would generate a trace warning
      innerPubWeb.Web.Dispose();
    }
  }
}

Update 1/29/2009: Roger Lamb has added this GetVariation() guidance to his Dispose Patterns by Example.

PowerShell Dispose-All

With all the talk of Dispose safety in object model code, it’s easy to forget that those same objects need to be cleaned up in PowerShell as well. Sure, SharePoint will clean everything up when the thread ends, but my PowerShell doesn’t get closed all that often, leaving open SPRequests for extended periods of time. To combat that, I wrote a quick function that will clean up all my IDisposable variables for me:

function Dispose-All {
    Get-Variable -exclude Runspace |
        Where-Object {
            $_.Value -is [System.IDisposable]
        } |
        Foreach-Object {
            $_.Value.Dispose()
            Remove-Variable $_.Name
        }
}

For some reason, I don’t think it would be a good idea to Dispose() the current Runspace… (Edit: Apparently I define $Runspace in a long-forgotten part of my profile script, so don’t be worried if you don’t have that defined.)

Of course this isn’t foolproof, as variables can be overwritten and not all IDisposables will have been assigned to variables in the first place, but you could certainly adapt your shell behavior to make the most out of a catch-all cleanup like this.