SPDisposeCheck is a great utility for checking the disposal patterns you’re using in your SharePoint solution. Believe me, even experienced practitioners make mistakes; object disposal in SharePoint is hard.
Now, every time someone says that SPDisposeCheck is great, someone posts back that it isn’t perfect. They’re right; it won’t pick up every invalid disposal pattern – but it’s pretty good. Given the choice of it or no check, it’s a clear winner.
I’m not going to describe it much – there are good articles already out there about the problem, and how to use SPDisposeCheck. It’s also well worth installing the latest version, which integrates with Visual Studio. That’s brilliant – you can run a test over your assemblies with each build.
However, recently I took over someone else’s code; SPDisposeCheck proved invaluable, but quirky too. This post will examine some of the practical issues that I found with using it.
SPDisposeCheckIgnoreAttribute
SPDisposeCheck is quite upfront about sometimes logging false positives:
This tool may report false positives. False positives are reported errors where there is not an actual error. This can happen because SPDisposeCheck does not currently follow variables beyond the current methodCall. For instance, if foo was a disposable type and it was passed to bar in which dispose was called.
May produce false positives if types are disposed outside the current scope. In other words, if you assign a disposable type in one method and then pass it to another method or perhaps a delegate in which it is subsequently disposed then this will produce a false positive.
False positives may also be produced depending on the way the type was created. …
However, they offer a way of dealing with these – you can add an attribute infront of your function that tells SPDisposeCheck to ignore that error type from that method.
[SPDisposeCheckIgnore(SPDisposeCheckID.SPDisposeCheckID_000, "Context Web does not need disposed.")]
The definition of this attribute is in the documentation in the SPDisposeCheck solution, and you can read more about it here. It’s neat – it gives use a way to get to a ‘clean’ bill of health after each compile, by ignoring possible problems that we’ve investigated. Or does it?
SPDisposeCheckIgnoreAttribute and Namespaces
The documentation for SPDisposeCheck says:
You can safely change the namespace of SPDisposeCheckIgnore to match your project to avoid additional using statements in your source files.
This is incorrect. Using Reflector I decompiled SPDisposeCheck to find why it wasn’t working and saw:
This is clearly checking for the SPDisposeCheck namespace. When I returned the namespace in my assembly to SPDisposeCheck it started working correctly.
SPSite.RootWeb
There appears to be a fault in SPDisposeCheck where, if you use the RootWeb property, you’re damned if you dispose the SPWeb you get back, and damned if you don’t. Essentially, SPDisposeCheck raises a warning either way.
Stefan Gossner describes it here.
Note that the correct approach is not to dispose the SPWeb that’s created – disposing of the SPSite you used to get it will dispose of the rootweb object too.
SPDisposeCheckIgnore and RunWithElevatedPrivileges
The code I’d inherited used RunWithElevatedPrivileges – a lot. The calls were of the form:
SPSecurity.RunWithElevatedPrivileges(delegate() { using (SPSite site = new SPSite(item.Web.Url)) { SPWeb rootWeb = site.RootWeb; // Do something } }
This causes a problem, even though the pattern is correct. The call to RootWeb is bound to log an issue (see above). However, our anonymous method can’t have an attribute added to it. There are two options:
- Use a named delegate, rather than an anonymous one (ugly).
- Wrap the call to site.RootWeb in a utility function, which I can then add the ignore attribute too (Annoying, but simple)
Here’s what I used:
namespace SPDisposeCheck { public class DisposeUtils { [SPDisposeCheckIgnore(SPDisposeCheckID.SPDisposeCheckID_140, "RootWeb does not need disposed.")] public static SPWeb GetRootWeb(SPSite site) { return site.RootWeb; } } }
I then changed my delegates to use this function, rather than calling site.RootWeb directly. That solved that issue. It then struck me that if I replaced all calls to site.RootWeb – even outside elevated privileges delegates – to use the method then I’d only have to ignore this particular SPDisposeCheck once.
End result – I could see the actual errors in the code I’d been given, and fix their patterns. The application now compiles without any SPDisposeCheck violations – but it will error and tell me when I introduce one. From 150-odd warnings to zero – success!
Wow I love the simple fix at the end, great idea.