What are the dangers of using a static class that contains your SPWeb object?

Question

When creating my SharePoint 2010 solutions, I often create a static class that contains helper methods and lists, documents, list items, etc that I use frequently. What would the dangers of putting my SPWeb object in there? Or SPSite for that matter?

My static class would contain a method like:

    public static SPWeb webObject;

    public static SPWeb WebObject()
    {
        if (webObject != null)
            return webObject;
        else if (SPContext.Current != null)
        {
            webObject = SPContext.Current.Web;
        }

        return webObject;
    }

That way I can populate the webObject at any time, or just call on the method which would return the webObject or SPContext.Current.Web if webObject is null.

So instead of writing my code like:

SPWeb web = site.RootWeb;
SPList list = web.Lists[listGuid];

It would look like:

SPList list = SharePointState.WebOject().Lists[listGuid];

Thus saving me an extra line of code and I wouldn't have to constantly be initializing a SPWeb object.

Now, disposing of the SPWeb object would make this idea pointless since the static method would just be recreating the SPWeb object every time I called on it. Would this crazy idea drain resources and make my solution perform slowly? What are some of the dangers of doing this?

Answer 1

This is bad for 3 reasons:

1. It will introduce a bug. static means it is shared across all threads for the life of the AppDomain. When user A goes to /web1 and user B goes to /web2 a second later, this code will try to execute as if it's in /web1.

2. It could cause a memory leak. Holding a web any longer than absolutely necessary is almost always a really bad idea. In this case you're only holding one web (though see above for why that's bad) - but it reflects a misunderstanding about how SPWeb actually works. You should open them as late as possible and dispose them as early as possible; and if you aren't the one calling Open(), then leave them the hell alone!

3. It will cause performance problems and unexpected errors. Holding SPWeb objects across threads is not supported. You will start to see errors in the ULS logs saying something like "Unable to close SPRequest; you may be opening and closing Webs on different threads." I have seen this cause all kinds of bizarre problems, like causing SQL Server load to spike and sites to become unresponsive for minutes.

Lessons learned:

• SPContext.Current already knows how to track and manage which is the "right SPWeb" for the current calling context. Trust it.
• If you didn't open the web, don't close it. (More generally, if you didn't create the IDisposable object - it was handed to you - then don't dispose it.
• Always wait until the last possible moment before opening a SPWeb, and do what you need to do with it quickly, and then close it.

Answer 2

Someone has already done what you want, in a much better way. In fact, you're using it in your example code:

public static SPWeb WebObject()
{
    if (webObject != null)
        return webObject;
    else if (SPContext.Current != null)
    {
        webObject = SPContext.Current.Web;
    }

    return webObject;
}

You are using SPContext.Current.Web as a static property here!

Your next two code examples are not at all equivalent

SPWeb web = site.RootWeb;
SPList list = web.Lists[listGuid];

This acts on the root web if a site collection

SPList list = SharePointState.WebOject().Lists[listGuid];

This acts on the current web site, regardless of its depth in the site hierarchy.

You can already use the following construct:

SPList list = SPContext.Current.Web.Lists[listGuid];

You are not only trying to reinvent the wheel, you're starting it off as a square. SharePoint already has its own garbage collector process that keeps track of SPWeb and SPSite objects and will dispose them if it thinks they've been around for too long. It is possible however to manage scope in a manner similar to what you are doing, but you should go up one more level of abstraction and implement repositories that properly handle SP* objects. Those repositories you can make static, singletons, whatever suits your patterns.

Know also that objects like SPContext.Current.Web and SPSite.Rootweb are already partially cached. Using the same one frequently over a normal browsing session does not mean a complete teardown and setup every time.

Answer 3

Bad idea, a static class in a web based application (like SharePoint) means that whoever visits that site first will trigger the creation of the static object. Any user after that will reuse the same object, meaning that any subsequent user would try to use the first user's context. Which is not only a bad idea (raagrding security for instance), but probably not going to work, seeing as HTTP is basically stateless so any SPContext would be disposed of after each request, resulting in a memory leak for the next request ("SPException: trying to use an spweb object that has been closed or disposed and is no longer valid").

NEVER close, dispose or reuse an SPContext object (pretty much the rule of thumb is: if you don't "own" the object, i.e. you have not instantiated it yourself using for instance a constructor, don't dispose / close it. I suggest using SPDisposeCheck regularly to check your code for possible memory leaks!

Answer 4

I agree with the last poster's point that if you don't "own" an object you shouldn't dispose of it. However, I completely disagree with the idea of avoiding a static class.

The term "static" really just means shared. Shared instances of objects are used all over the place in ASP.NET. The Cache API and Application state are perfect examples. There is absolutely nothing wrong with using static or shared instances where applicable.

One great example is the idea of an HttpContext. That context will be exactly the same for every single user who visits a site. You would never want that to be anything other than shared; it necessarily MUST be the same for every single user.

In my book, static objects are just fine in a Web Application. I will say that you should avoid using the Singleton model; that comes with all kinds of headaches. But a simple static object is fine. Christ... any extension methods you create are part of a static class!

As long as they are used with purpose and not just for the sake of using something that's "shared" a static class is totally acceptable.