‘using’ IDisposable objects

Artikkeli
12/28/2010

This isn’t fully game related, but I see so many people posting code on forums that looks like this:

 Stream stream = storageContainer.CreateFile("MyFile.txt");
StreamWriter writer = new StreamWriter(stream);
writer.Write("Stuff");
writer.Close();
stream.Close();

I cringe at this sight.

IDisposable is an interface that objects leveraging native handles or resources will almost always implement to ensure the native resources are released properly and deterministically. If any of the code in there were to cause an exception, you’d risk not releasing those resources at that point. At best you’d see them released when a garbage collection occurred and at worst they wouldn’t get cleaned up at all.

Now, most people will respond with “but there’s so little going on there, what would throw an exception?” and that’s valid for this example, but a lot of games do a lot more in there. Things like serializing (or deserializing) objects, parsing strings into numbers, and so on. There are lots of places where file IO, state saving/loading, and other Stream manipulation can throw, so it’s always better to be on the side of safety rather than stubbornness.

Anytime a type implements IDisposable, that should be your hint to leverage the ‘using’ statement. In an unfortunate case, the ‘using’ keyword is not only used for including namespaces but also for scoping IDisposable objects so that they are guaranteed to be disposed. To reuse the example above, here’s the correct way to write that:

 using (Stream stream = storageContainer.CreateFile("MyFile.txt"))
{
    using (StreamWriter writer = new StreamWriter(stream))
    {
        writer.Write("Stuff");
    }
}

Not only does this help scope your objects to prevent using either the stream or writer objects outside of their respective blocks, but it means that even if the code in the middle of the ‘using’ block throws an exception, the objects created in the ‘using’ statements are guaranteed to be disposed. Basically the compiler takes the above code and turns it into this:

 {
    Stream stream = storageContainer.CreateFile("MyFile.txt");
    try
    {
        StreamWriter writer = new StreamWriter(stream);

        try
        {
            writer.Write("Stuff");
        }
        finally
        {
            if (writer != null)
                ((IDisposable)writer).Dispose();
        }
    }
    finally
    {
        if (stream != null)
            ((IDisposable)stream).Dispose();
    }
}

(And yes, those extra curly braces are intended; they are used to scope the ‘stream’ object so it cannot be used outside of the ‘using’ block.)

As you can see this is much safer code than the first snippet because it removes the possibility of not disposing those objects and thus ensures that native resources will be properly released when you expect rather than when the next GC occurs or when your app shuts down.

So please, if you find yourself writing code similar to the first snippet, please stop doing that and start ‘using’ IDisposable objects properly. Winking smile

Comments

Anonymous
December 28, 2010
Excellent article. I cringe at code that doesn't use the 'using' blocks. On another note, you can also stack the 'using' statements like this... using (Stream stream = storageContainer.CreateFile("MyFile.txt")) using (StreamWriter writer = new StreamWriter(stream)) { writer.Write("Stuff"); } Not as many curly braces but essentially the same.
Anonymous
December 28, 2010
Of course, if you're needing to return said IDisposable, you must not use using. The calling code has to take responsibility for it.
Anonymous
December 28, 2010
Of course, if you're needing to return said IDisposable, you must not use using. The calling code has to take responsibility for it.
Anonymous
December 28, 2010
One of the downside (in my opinion) of this method is that you cannot force your code's user to behave nicely and use this syntax, or call Dispose() himself. I tend to add an additional security measure that can be found here: msdn.microsoft.com/.../fs2xkftw(VS.80).aspx
Anonymous
December 28, 2010
The only thing I'd add is that a 'using' statement doesn't replace a try-catch block and magically whisk any exceptions away; it just takes care of disposing any unmanaged resources (like streams) in the event of an exception. So remember to embed those 'using' statements inside of try-catch blocks (assuming you're handling the exceptions there rather than letting them bubble up to some other code) or else you'll be in for an unpleasant surprise one day when you finally do get an exception.
Anonymous
December 28, 2010
One other thing, actually. Nested usings violate CA2202 ( msdn.microsoft.com/.../ms182334.aspx ). The design variation suggested there is sound in the event of a bad implementation of IDisposable. Testing for bad Dispose implementations is as simple as calling Dispose before exiting the using block and seeing if the code throws an ObjectDisposedException (a glimpse through the lens of Reflector or the like would also be useful if it is possible to examine the assembly/ies in question). If there are problems and you cannot fix the Dispose method in question (e.g. 3rd party library-only framework where the vendor refuses to fix it or cannot get a patch out before you need to ship) then you can go with the ugly but functional "outer try-finally" pattern given in that particular CA rule.
Anonymous
December 28, 2010
They're NOT CURLY braces - they're BRACES! There are no not-curly braces!
Anonymous
December 28, 2010
MikeBMcL - I agree with the dissenters on that MSDN page and the author of this article: nested 'usings' encourages good coding practice by keeping it simple, readable, and consistent. CA2202 makes no sense. If a dispose method is incorrectly implemented and multiple disposes causes a problem, then that's a bug in the library and the library should be fixed. As you said, it's very easy to test for, so unit testing the class's dispose is a no-brainer. The outer try-finally style is atrocious. BigFredd - braces are for teeth, curly braces are for coders. ;-)
Anonymous
December 29, 2010
I also love the pattern when you implement IDisposable on a token object so that the Dispose method would cancel an action or unregister an event. Something like var token=BindToEvent() and then, when you want to stop handling the event, token.Dispose().
Anonymous
December 29, 2010
Thank you, this was very helpful. I am now updating my current project to make use of this, but since I am doing this in VB, I thought I would add a translation. Using Stream As Stream = storageContainer.CreateFile("MyFile.txt") Using writer As New StreamWriter(Stream) writer.Write("Stuff") End Using End Using
Anonymous
December 29, 2010
There is actually a problem with this and all the posts making use of nested 'using' statements. If you use static code analysis you will be warned of nesting a 'using' statement that uses the object created in the previous 'using'. CA2202: msdn.microsoft.com/.../ms182334.aspx The issue is because the second object usually takes ownership of the first. When disposing the second/nested object (by going out of scope) the first object will get it's dispose method called as well. When the first object goes out of scope the dispose method will be called again. This is not usually a problem when coding with built-in .NET types but generally isn't good for business, as long as MS keeps to their own standard... from the rule page itself "A correctly implemented Dispose method can be called multiple times without throwing an exception. However, this is not guaranteed and to avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object." I've been bitten by this issue in Mono and it will probably happen again when using third party libraries that don't implement IDisposable correctly. Better safe than sorry!
Anonymous
December 30, 2010
I agree that you should at least be careful about bad implementations and test it accordingly, then you could proceed to either fix the 'bug' if you can or go for the try-finally alternative. You can never be sure unless it's a .NET type that the 2nd object's Dispose will take care of the 1st one. Also, just in case, especially if you can't use the using because you need to return the object, you could implement the special 'finalize' method so that it calls the Dispose if the object is not disposed already. In the end, nothing can replace a good testing strategy, though.
Anonymous
December 31, 2010
Regarding the VB translation, for brevity we can combine them into one Using statement: Using stream As Stream = storageContainer.CreateFile("MyFile.txt"), writer As New StreamWriter(stream) writer.Write("Stuff") End Using
Anonymous
January 03, 2011
This is why C# is still a toy - its stubborn refusal to have determinate destructors. C++/CLI can handle this much better. It should not be necessary to know if native resources need to be released. Resources should be released when no longer necessary - period.
Anonymous
January 04, 2011
It's not just Mono or third-party libraries which have problems implementing IDisposable - try disposing of a ReaderWriteLockSlim twice and see what happens. connect.microsoft.com/.../readerwriterlockslim-does-not-implement-idisposable-properly Cygnus: If you want a language with determinate destructors like C++/CLI, then why don't you try C++/CLI? I hear it can handle this much better. :o)
Anonymous
January 05, 2011
Is there a tool that can warn us when we new up instances of IDispsable without using statement? E.g. Reshaper, JustCode, CodeRush, Code Analysis... It would be nice to tell us if we create of a "Type" that implements IDisposable and that is not used with a using statement...
Anonymous
January 10, 2011
Peter Gfader, CodeRush Pro has "code issues" feature that highlights undisposed locals and has a fix to automatically apply a using statement on code in question.

Jaa

‘using’ IDisposable objects

Comments

Lisäresursseja