What happens to an IDisposable object created in the middle of a statement that I cannot explicity call Dispose() on?

Question

Say I'm working with Sharepoint (this applies to other object models as well) and in the middle of my statement, I call a method, in this case "OpenWeb()", which creates an IDisposable SPWeb object. Now, I cannot call Dispose() on the SPWeb object because I don't have the reference to it. So do I need to be concerned about this leaking memory?

SPUser spUser = SPControl.GetContextSite(HttpContext.Current).OpenWeb().SiteUsers[@"foo\bar"];

I know that I could have broken up the statement into multiple lines and get the SPWeb reference to call Dispose:

SPWeb spWeb = SPControl.GetContextSite(HttpContext.Current).OpenWeb();
SPUser spUser = spWeb.SiteUsers[@"foo\bar"];
spWeb.Dispose();

Please keep in mind that my question is not about aesthetics, but more about what happens to the IDisposable object that I cannot explicitly call Dispose() on, since I don't have the reference.

Sorry about not being clear enough when I first asked the question. I've since rephrased it. Thanks for all the responses so far.

Answer 1

No one seems to have thrown this in yet:

If your object requires a disposer (i.e. takes hold of resources that must be released) then you should implement a finalizer that calls the disposer's method.

In the disposer you can add the line:

System.GC.SuppressFinalize(this)

Which will prevent finalization if you call the disposer. That way you can use your object nicely if neccessary, but guarantee that it cleans up after itself via the finalizer, (which is the whole reason that C# does have a finalizer).

Answer 2

"what happens to the IDisposable object that I cannot explicitly call Dispose() on?"

In general you can call Dispose (either implicitly with a using statement or explicitly) on all disposable objects, however in the hypothetical scenario where you can not, it depends on the way the object was implemented.

In general, the .Net objects will follow pattern along these lines. The pattern is to define a finalizer that cleans stuff up in case dispose is not called and then have dispose suppress the finalizer. Which reduces memory load and gives the GC less work to do.

Amongst the many problems with calling Dispose from the finalizer, is that you are turning a single threaded problem into a multithreaded problem, the finalizer will run on a different thread which can expose some very subtle and hard to catch bugs. Also, it means you will be holding on to unmanaged resources for longer than expected (Eg. you open a file, forget to call close or dispose and next time you go to open it its locked)

Bottom line is, it is a best practice to dispose all disposable objects, otherwise you can introduce weird and complex bugs. With the caveat that some frameworks, like sharepoint, return shared instances of objects that should not be disposed according to the documentation.

I usually find the code much more readable when I dispose my objects with the "using" pattern. The problem with calling Dispose explicitly (object.Dispose()) is that it can be hard to trace where the object was allocated and it is very easy to forget. You can not forget to close the curly bracket of the using statement, the compiler will complain :)

EDIT / GOTCHA

According to MS documentation You should not call dispose on references to share point shared objects which are returned by GetContextSite. So, you should be extra careful here.

See this answer for a safe sharepoint pattern you should use.

However, if you have a reference to a shared resource, such as when the object is provided by the GetContextSite method in a Web Part, do not use either method to close the object. Using either method on a shared resource causes an Access Violation error to occur. In scenarios where you have a reference to a shared resource, instead let Windows SharePoint Services or your portal application manage the object.

Answer 3

Many answers here assume it's important only that Dispose is called eventually. When working with SPSite and SPWeb, however, you definitely want to call Dispose() as soon as you can. Determining when you should is often tricky, but there are many good references available to help answer that question.

As to why this is the case, Stefan Goßner provides a great summary here:

Each SPWeb and SPSite object holds a reference to an SPRequest object which holds a reference to a SharePoint COM object that is responsible to communicate with the backend SQL server.

Disposing a SPWeb object will not actually remove the SPWeb object from memory (actually the .NET framework does not allow to remove any object from memory in a deterministic way) but it will call a method of the SPWeb object which causes the COM object to close the connection to the SQL server and to release its allocated memory.

That means the connection to the backend SQL server will remain open from the moment the SPRequest object has been created till the SPWeb object is disposed.

The best practice for your code sample would look like this:

SPSite contextSite = SPControl.GetContextSite(HttpContext.Current);
using (SPWeb spWeb = contextSite.OpenWeb())
{
  SPUser spUser = spWeb.SiteUsers[@"foo\bar"];
  // Process spUser
}
// DO NOT use spUser
// DO NOT dispose site from HttpContext

Note that it is not safe to use SP objects like SPUser, SPList, etc. after their parent SPWeb has been disposed.

Answer 4

As it has been said by some so far: you must never dispose objects you dont create. So in your example, you should not dispose either the SPWeb or SPSite object!

That will destroy the current SPRequest object. It might seem like it is still working, but if you for example add new web parts later, or try opening the tool pane for your web part, you will get all sorts of strange errors.

As it has already been said, it is a must to dispose instances of SPWeb and SPSite that you create yourself (new).

This can be done either with using() or with try/finally (which is the way a using() statement will show up in your MSIL code anyway). If you use try/finally it is best practice to check for null on your SPWeb/SPSite instance, and check for SPWeb first, as SPSite automatically will dispose your SPWeb.

Another important thing to remember is when looping SPWebCollections like AllWebs or Webs is to dispose your subwebs as you loop through them. If there are alot of sub webs, and you are running on 32-bit hardware with limited memory potential, you can fill up your memory with SPRequest objects very fast indeed. This will cause sluggish performance since it will cause your application pool to recycle regularily.

When that is said, it is also a good practice not to combine calls like you do in your code example. It is hard to read, and if you were working with an SPWeb that you should dispose, you couldnt! These kind of memory leaks are the hardest to spot, so dont do it ;-)

I can reccomend Roger Lamb's blog for details: http://blogs.msdn.com/rogerla Also some techincal details on Stefan Goßner’s blog: http://blogs.technet.com/stefan_gossner/archive/2008/12/05/disposing-spweb-and-spsite-objects.aspx

hth Anders

Answer 5

From http://msdn.microsoft.com/en-us/library/aa973248.aspx Best Practices: Using Disposable Windows SharePoint Services Objects:

If the SPSite object is obtained from SPControl.GetContextSite, the calling application should NOT dispose of the object. Because the SPWeb and SPSite objects keep an internal list that is derived in this way, disposing of the object may cause the SharePoint object model to behave unpredictably.

Answer 6

I would suggest splitting the line and using Dispose. If an object implements IDisposable, you have to assume that it requires disposing and hence use it in a using block.

using (SPWeb spWeb = SPControl.GetContextSite(HttpContext.Current).OpenWeb())
{
    SpUser spUser = null;
    if (spWeb != null)
    {
        spUser = spWeb.SiteUsers[@"foo\bar"];
    }
}

By doing this you get to Dispose the object and also handle errors in your OpenWeb() call that is opening an external resource.

Answer 7

Leaking memory? No, you shouldn't be worried about it, assuming that the implementation of IDisposable adheres to the class library guidelines, as the next garbage collection should clean it up.

However, it does reveal an error in your code, as you aren't properly managing the lifetime of the implementations of IDisposable that you work with (I elaborate on this issue at http://www.caspershouse.com/post/A-Better-Implementation-Pattern-for-IDisposable.aspx). Your second code block is a good first step, but you don't guarantee the calling of Dispose if the call to SiteUsers fails.

Your revised code would look like this:

// Get the site.
var contextSite = SPControl.GetContextSite(HttpContext.Current);

// Work with the web.
using (SPWeb web = contextSite.OpenWeb())
{
  // Get the user and work with it.
  SPUser spUser = web.SiteUsers[@"foo\bar"];
}

Answer 8

The following is more idiomatic and reads better as well:

using (SPWeb spWeb = SPControl.GetContextSite(HttpContext.Current).OpenWeb())
{
    SPUser spUser = spWeb.SiteUsers[@"foo\bar"];
}

Answer 9

You should explicitly (or implicitly via using statement) call Dispose method . Another reasons to split code in multiple lines are:

• readability
• it's easier to debug

Dispose method can be executed in finalizer, but it is safer to call it yourself.