Don’t dispose of objects that you don’t own

phillips.joshua

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;
}

.csharpcode, .csharpcode pre { font-size: small; color: black; font-family: consolas, “Courier New”, courier, monospace; background-color: #ffffff; /*white-space: pre;*/ } .csharpcode pre { margin: 0em; } .csharpcode .rem { color: #008000; } .csharpcode .kwrd { color: #0000ff; } .csharpcode .str { color: #006080; } .csharpcode .op { color: #0000c0; } .csharpcode .preproc { color: #cc6633; } .csharpcode .asp { background-color: #ffff00; } .csharpcode .html { color: #800000; } .csharpcode .attr { color: #ff0000; } .csharpcode .alt { background-color: #f4f4f4; width: 100%; margin: 0em; } .csharpcode .lnum { color: #606060; }

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! .csharpcode, .csharpcode pre { font-size: small; color: black; font-family: consolas, “Courier New”, courier, monospace; background-color: #ffffff; /*white-space: pre;*/ } .csharpcode pre { margin: 0em; } .csharpcode .rem { color: #008000; } .csharpcode .kwrd { color: #0000ff; } .csharpcode .str { color: #006080; } .csharpcode .op { color: #0000c0; } .csharpcode .preproc { color: #cc6633; } .csharpcode .asp { background-color: #ffff00; } .csharpcode .html { color: #800000; } .csharpcode .attr { color: #ff0000; } .csharpcode .alt { background-color: #f4f4f4; width: 100%; margin: 0em; } .csharpcode .lnum { color: #606060; }

0 comments

Discussion is closed.

Feedback usabilla icon