Welcome to MSDN Blogs Sign in | Join | Help

News

  • These postings are provided "AS IS" with no warranties and confer no rights. All code and tools presented are done so under the Microsoft Public License.
Don’t dispose of objects that you don’t own

In concurrent programs, race conditions are a fact of life but they aren’t all bad.  Sometimes, race conditions are benign, as is often the case with lazy initialization. 

The problem with racing to set a value, however, is that it can result in multiple objects being instantiated when only one is needed.  Take the LazyInitializer class that shipped in Visual Studio 2010 Beta 1, for example.  The method EnsureInitialized(ref T target, Func<T> valueFactory) accepts a ByRef value and a function that produces a value.  If target is null, EnsureInitialized will execute valueFactory and set target to the return value.  If multiple threads happen to overlap calls to EnsureInitialized, which is likely to be a rare occurrence, these threads may both initially see target as null and then execute valueFactory.  One thread will win the race and will get to set target

In Beta 1, we thought we’d be really smart by disposing of the objects that were created by threads that lost the race, if they implemented IDisposable.  Turns out that’s a pretty bad thing to do.  With lazy initialization, we assumed that most of time valueFactory would be creating a new value but that isn’t always the case.  It’s quite possible that a valueFactory could return an object that has already been created, in which case, we’d be disposing of an object we didn’t own. 

We’re making a couple of changes in the Parallel Extensions to ensure that we generally don’t dispose of object we aren’t sure we own and, moving forward, any new APIs that do eagerly instantiate objects that might not be used will not dispose of said objects. 

Now you might say, “why?  Ninety-percent of the time I am creating a new object with my value factory and it might be a really heavy object like a file or wait handle!”  We hear you, but typically, once an object is disposed, there is no bringing it back and so by disposing of these objects we don’t allow anyone to opt out of that behavior if need be.  If we don’t dispose, the GC will still cleanup your unused objects and if you really need to dispose of an object manually, there is a work around, i.e.

ManualResetEvent theEvent = null;
ManualResetEvent tmpEvent;
LazyInitializer.EnsureInitialized(ref theEvent, 
     () => { return tmpEvent = new ManualResetEvent(false); });
if (tmpEvent != theEvent) tmpEvent.Close();

In fact, you could easily create a small wrapper function to do the work for you, i.e.

public static T EnsureInitializedWithDispose<T>(ref T target, Func<T> function) 
    where T : class, IDisposable
{
    T tmp = default(T);
    T actual = LazyInitializer.EnsureInitialized(ref target, () => tmp = function());
    if (actual != tmp) ((IDisposable)tmp).Dispose();
    return actual;
}

And the issue isn’t just about race conditions.  In general, we should never dispose of an object we didn’t explicitly create.  BlockingCollection<T> in Beta 1, for example, will dispose of it’s underlying collection if the BlockingCollection<T> itself is disposed.  Again, this is bad news if you only were using the BlockingCollection<T> as a temporary wrapper.  In future releases of Visual Studio 2010, this will be corrected.

So when using these APIs, and any other API that may consume or create an IDisposable object, think about what that API is doing with the object and if you need to clean up resources manually.  If you’re not sure, the finalizer should take care of most of your problems.  In the rare situations where immediate cleanup is important, make sure you keep track of the objects and dispose of them properly.  Also, when designing your own libraries, don’t dispose of objects you don’t own!

Posted: Wednesday, June 24, 2009 11:39 PM by phillips.joshua

Comments

Joe Albahari said:

Shouldn't EnsureInitialized be overloaded to provide this disposal behaviour as an option?

If you make this an option, it will yield greater benefit than just convenience. When people see the disposeUnusedObject parameter in IntelliSense, they'll be forced to think about the problem - which is surely a good thing!

Otherwise, all but the most astute people will be unaware that the problem exists and will leave undisposed objects at the mercy of the GC's schedule. Which is OK with wait handles (which have a low OS burden), but not so cool with file handles (which can block access to the file!)

# June 25, 2009 11:15 PM

Kristian Freed said:

Nice posting!

I think ownership of objects is something people spend much too little time thinking about (and documenting!) when it comes to garbage collected languages in general. In C++ for example, you need to be explicitly aware of who owns an object much more often, because someone is responsible for deallocating it at some point. Removing the need to explicitly free objects from memory also gives the illusion that all other questions of ownership (May I change this object I received from somewhere else, or do I have to make a copy? Can it change while I'm using it?  May/should I dispose of it when I'm done with it myself?) have been resolved automatically as well.

# June 26, 2009 5:14 AM

phillips.joshua said:

Hi Joe,

Ideally, yes.  This is exactly what we had hoped to do.  Unfortunately, the harsh reality of shipping a product is that sometime you have to, well, ship.. And to do that, we have to stop churning code at some point.  Given that this API is really advanced and we don't expect many mainstream developers to use it (they'd use Lazy<T> instead) we decided not to prioritize adding an overload.  Those that will use it, are likely to be very concerned with things like file handles and make sure they're disposed properly.

Thanks!

Josh

# June 26, 2009 11:17 AM

Tom Kirby-Green said:

I would concur with Kristian's point, a lot of developers seems to mentally broaden the GC's remit from 'memory' to 'general responsibility' collector. Gosh how I wish I could declare a const reference to a object in C# ...

# June 26, 2009 11:34 AM

Nicholas Paldino [.NET/C# MVP] said:

Shouldn't the call to compare be to ReferenceEquals on Object?  If for some reason, compare-by-value semantics has been implemented in the equality operator (which best practice indicates you should implement != as well), then there is an issue of disposing the wrong object here.

# July 2, 2009 12:21 PM

phillips.joshua said:

Hi Nicholas,

Good question. In most cases, that would be true, but in this case, the restrictions on generics actually helps(?) out here.  

In C# overloading operators on generic types is not supported and thus inside of the generic method, != will not resolve to the overriden implementation.  You do have a good argument for readability, however!

Josh

# July 6, 2009 9:12 PM

Duarte Nunes said:

Hi,

Indeed, object ownership is an issue with the disposable pattern, which is why the right thing to do is indeed to let the user deal with object disposal (or provide an explicit overload).

Also, I was looking at the beta 1 code for LazyInitializer and noticed that you use an acquire barrier on the public methods, but I can't seem to figure out what it's for, with my understanding of the memory model (is there going to be a formal memory model specification, based on Prism?).

Thanks!

Duarte

# August 6, 2009 11:55 AM

phillips.joshua said:

Hi Duarte,

We read a dummy volatile variable to prevent potential memory reordering in IA64.  

Thanks!

Josh

# August 6, 2009 2:56 PM

Duarte Nunes said:

Hi Josh,

Thanks for the answer! I guess you mean the potential load-load reordering. Does this mean the memory model for 4.0 that we developers must address is the weakest (i.e., IA64's)?

Thanks again,

Duarte

# August 6, 2009 3:26 PM

phillips.joshua said:

That, of course, depends on the architecture(s) you are targeting. :)  

Josh

# August 6, 2009 3:33 PM
Leave a Comment

(required) 

(required) 

(optional)

(required) 

  
Enter Code Here: Required

Comment Notification

If you would like to receive an email when updates are made to this post, please register here

Subscribe to this post's comments using RSS

Page view tracker