Common Pitfalls with IDisposable and the Using Statement

Memory management with .NET is generally simpler than it is in languages like C++ where the developer has to explicitly handle memory usage.  Microsoft added a garbage collector to the .NET framework to clean up objects and memory usage from managed code when it was no longer needed.  However, since the garbage collector does not deal with resource allocation due to unmanaged code, such as COM object interaction or calls to external unmanaged assemblies, the IDisposable pattern was introduced to provide developers a way to ensure that those unmanaged resources were properly handled.  Any class that deals with unmanaged code is supposed to implement the IDisposable interface and provide a Dispose() method that explicitly cleans up the memory usage from any unmanaged code.  Probably the most common way that developers dispose of these objects is through the using statement.

The Using Statement

Microsoft provided the using statement to simplify the code required to properly dispose of an object.  Here is an example of how the C# compiler interprets the using statement:

public void AddToFile(string text)
{
    using (System.IO.FileStream file = new System.IO.FileStream("test.txt", System.IO.FileMode.Append))
    {
        byte[] encodedText = System.Text.Encoding.Default.GetBytes(text);
        file.Write(encodedText, 0, encodedText.Length);
    }
}
public void AddToFile(string text)
{
    {
        System.IO.FileStream file = new System.IO.FileStream("test.txt", System.IO.FileMode.Append);
        try
        {
            byte[] encodedText = System.Text.Encoding.Default.GetBytes(text);
            file.Write(encodedText, 0, encodedText.Length);
        }
        finally
        {
            if (file != null)
            {
                file.Dispose();
            }
        }
    }
}

That looks pretty straight forward and is simple to use, right? Well, there are a few use cases to watch out for with the using statement that could result in a memory or resource leak of some kind.

#1: Object Initializers

Object initializers are a nice feature and they can cut down on the amount of code needed to properly setup new instances of data models. However, they can also cause problems if they are used inside of the using declaration.  Here is an example that introduces the possibility of the FileStream not getting disposed properly:

public void ReadDataAtBytePosition(long bytePosition)
{
    using (FileStream stream = new FileStream("test.txt", FileMode.Open) { Position = bytePosition })
    {
        int byteValue = stream.ReadByte();
    }
}

At first glance it looks simple and innocent, but if we remember how the compiler interprets the using statement then we can see the potential for a problem. Here is the interpreted version:

public void ReadDataAtBytePosition(long bytePosition)
{
    {
        FileStream stream = new FileStream("test.txt", FileMode.Open);
        stream.Position = bytePosition;
        try
        {
            int byteValue = stream.ReadByte();
        }
        finally
        {
            if (stream != null)
            {
                stream.Dispose();
            }
        }
    }
}

Notice how there is a property setter call in between the constructor and the beginning of the try block. If the setter for the Position property throws an exception then the try/finally block will never execute and our previously allocated FileStream will not be disposed of. The best way to handle this case is to not use object initializers in the using declaration and instead call the property setter inside of the using block.

public void ReadDataAtBytePositionMid(long bytePosition)
{
    using (FileStream stream = new FileStream("test.txt", FileMode.Open))
    {
        stream.Position = bytePosition;
        int byteValue = stream.ReadByte();
    }
}

#2: Nested Constructors for IDisposable Objects

Some objects in .NET will allow you to create wrappers around another object, and in most cases when you dispose the wrapper object it will in turn dispose the inner object.  A commonly used version of this pattern is with the various StreamReader, StreamWriter, and Stream types. There is not a problem with wrapping objects that implement IDisposable, but just like the use of object initializers in the using declaration there is a problem with nesting the constructors.  Here is an example:

using (StreamReader reader = new StreamReader(new FileStream("test.txt", FileMode.Open, FileAccess.Read)))
{
    string entireFile = reader.ReadToEnd();
}

This will be interpreted into the following code:

{
    FileStream tmp = new FileStream("test.txt", FileMode.Open, FileAccess.Read);
    StreamReader reader = new StreamReader(tmp);
    try
    {
        string entireFile = reader.ReadToEnd();
    }
    finally
    {
        if (reader != null)
        {
            reader.Dispose();
        }
    }
}

Notice the absence of any call to tmp.Dispose(). With this case, disposing of the StreamReader will dispose of the underlying FileStream and there would be no issue, but what would happen if the constructor of the StreamReader threw an exception? In that case there would not be an instance of a StreamReader to dispose, which in turn would also mean that the FileStream would not get disposed and we would have a resource leak. We need to re-code this to take into account the possibility of a failure to construct an instance of the StreamReader. If we do that, then we would get the following code:

FileStream tmp = null;
try
{
    tmp = new FileStream("test.txt", FileMode.Open, FileAccess.Read);
    using (StreamReader reader = new StreamReader(tmp))
    {
        tmp = null;
        string entireFile = reader.ReadToEnd();
    }
}
finally
{
    if (tmp != null)
    {
        tmp.Dispose();
    }
}

The important part here is that we have to null out the temporary FileStream before we execute any code that might throw an exception. Now if the StreamReader fails to construct a new instance, then tmp will not be set to null and we will still properly dispose of the FileStream.

#3: Nested using statements

In the previous example you might have questioned why we did not just simply use two different using statements instead of adding the try/finally block.  There is nothing inherently wrong with nesting using statements. However, an issue arises when you nest using statements where the inner block will dispose of the outer block; This results in the outer block getting disposed of multiple times.  Although Microsoft does suggest that developers implement their Dispose() method in such a way so as to gracefully handle getting disposed of multiple times, there is no guarantee that every disposable object will follow that recommendation. In fact, a good example of this is the System.Xml.XmlTextWriter class. The Dispose() method for the XmlTextWriter assumes that there might be text still in the buffer and attempts to flush to the underlying stream. Because of this, calling Dispose() more than once will result in an ObjectDisposedException getting thrown from flushing to a stream that has already been disposed of.

If the two using blocks are not coupled together, a code block like the following is perfectly acceptable and will not cause any problems:

using (StreamReader reader = GetStreamReader())
{
    using (FileStream file = new FileStream("test.txt", FileMode.Append))
    {
        byte[] encodedBytes = System.Text.Encoding.Default.GetBytes(reader.ReadToEnd());
        file.Write(encodedBytes, 0, encodedBytes.Length);
    }
}

#4: Returning an IDisposable Object for Consumption

An important piece of dealing with IDisposable objects is determining who the owner is for the disposable object, since the owner of the object is ultimately responsible for disposing it.  A method that returns a disposable object for consumption somewhere else is not considered to be the owner once it successfully returns the object, and therefore does not need to worry about disposing the object after it is returned.  However, the method is considered the owner up until the point that it returns the object and needs to make sure that it properly disposes of it should anything prevent the method from returning. Let us look at the following example:

MemoryStream stream = new MemoryStream();
byte[] encodedText = System.Text.Encoding.Default.GetBytes(GetText());
stream.Write(encodedText, 0, encodedText.Length);
return stream;

The consumer is going to be responsible for disposing of the stream, but what if an exception is thrown in GetText() or GetBytes()? We need to tweak this code to make sure that the only two possible outcomes of executing this code are the stream being returned or the stream being disposed of. The following code will achieve this:

MemoryStream tmp = new MemoryStream();
try
{
    byte[] encodedText = System.Text.Encoding.Default.GetBytes(GetText());
    tmp.Write(encodedText, 0, encodedText.Length);
    MemoryStream stream = tmp;
    tmp = null;
    return stream;
}
finally
{
    if (tmp != null)
    {
        tmp.Dispose();
    }
}

This approach ensures that the stream will always get disposed of as long as the current method is the owner. Once the stream is returned by this method then it is the responsibility of the consumer to ensure that it does not leak.

#5: The Difference between Close() and Dispose()

Microsoft says that depending on the intended use of your class, naming the method Close instead of Dispose may make more contextual sense.  For instance, when dealing with database connections it is easier to understand what a method called close does more than a method called dispose.  That is why in some classes you will see a Close() method available but not a Dispose() method even though the class may implement the IDisposable interface.  In those cases the classes are explicitly implementing the interface and then providing a Close() method that logically makes more sense to the developer.  It is important to know that the Dispose() and Close() methods are both supposed to perform the same actions so it is not necessary to call both.  For instance, take a look at the following class that explicitly implements the interface:

class ExplicitDisposable : IDisposable
    {
        private System.IO.FileStream stream = new System.IO.FileStream("test.txt", System.IO.FileMode.Append);

        public void IDisposable.Dispose()
        {
            Close();
        }

        public void Close()
        {
            if (stream != null)
            {
                stream.Dispose();
            }
        }
    }

Given that class, all three of these consumers of it are identical.

using (ExplicitDisposable test = new ExplicitDisposable())
{
    Console.WriteLine(test.ToString());
}
ExplicitDisposable test = null;
try
{
    test = new ExplicitDisposable();
    Console.WriteLine(test.ToString());
}
finally
{
    if (test != null)
    {
        test.Close();
    }
}
ExplicitDisposable test = null;
try
{
    test = new ExplicitDisposable();
    Console.WriteLine(test.ToString());
}
finally
{
    if (test != null)
    {
        (test as IDisposable).Dispose();
    }
}

Because Close() and Dispose() execute the same code it is not necessary to call both of them as long as one of them is called.

In Conclusion

I hope that this post has highlighted a few areas to watch out for as you work with disposable objects.  While the using statement is nice and makes it pretty easy to handle disposable objects, there are still quite a few cases in which care needs to be taken to ensure that all of the possible outcomes are covered.  For more information about the IDisposable pattern, see the MSDN article at http://msdn.microsoft.com/en-us/library/b1yfkh5e(v=vs.110).aspx.

Comments

  1. rafaelsol says:

    Reblogged this on Unity and Game Codes.

  2. Although examples #1 and #3 are correct. You should always avoid initializing any property that has a risk of throwing until the using block and always treat an object that is disposable with care until it is handed off.

    Example #2 is incorrect. The example given is safe because the StreamReader does not throw exceptions unless:
    1) The stream is null (this will never happen, it will either throw or be created)
    2) The buffer size is <= 0 (you are not setting buffer size, so the defaults will be used and setting this to an invalid size is a bug anyhow)
    3) The stream cannot be read (Although CanRead is virtual, requesting this value from FileStream retrieves the value in a backing store so it will not throw). If you are opening a file and you aren't requesting the file access type, that is also incorrect and could cause this to throw here, but you are requesting the file access as Read, so it is safe. If read is denied the stream will not be created.
    4) You have an invalid or bad encoding type that throws an exception trying to retrieve the decoder, char count for the buffer size or the preamble for the encoding. (Since you don't specify this, it will use default UTF8 encoding which will not break unless the BCL itself is broken)

    In fact the only case it will have trouble is under a ThreadAbortException or StackOverflowException, but your case will also have trouble then. Even if the finally block is executed under these circumstances your solution could throw an exception if the abort happened just before the tmp is set to null since the disposal will happen twice and once the stream is handed to the reader you should not dispose the filestream object. If example #2 were unsafe Microsoft would not have added a file path parameter to stream reader that also creates a FileStream object in the constructor which would be effectively equivalent to the above.

    public StreamReader(string path);
    Internally Microsoft does this:
    new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, FileOptions.SequentialScan)

    This was added in .NET 3.0 long after FileStream and StreamReader were created in .NET 2.0 to simplify the use case of StreamReader because of the complexity of the situation which caused so many to abuse the stream handling.

  3. Although this is slightly off topic there is another caveat to IDisposable in connection with System.Threading.Tasks.Task objects. Everyone thinks you need to dispose of all IDisposable objects, but that is false in the case of Task objects.

    http://blogs.msdn.com/b/pfxteam/archive/2012/03/25/10287435.aspx

    • jpeters5392 says:

      Jeremy, thank you for your comments and the details that you provided. It is nice to get feedback from other developers.

      Your link to the post about disposing Task objects is an interesting read. I personally have not run into that but I will have to keep an eye out for it. Thank you for sending that.

      You are correct about my second example and I should update it to be a case where the StreamReader constructor can actually throw an exception. This example was just intended to be a somewhat contrived example based on code patterns that I have seen used in production to describe the danger of constructing an IDisposable object directly in a call to a constructor. While the StreamReader(string path) constructor is probably the best approach to solve this specific use case, I feel that the example is still valid for other types of nested disposable objects.

      However, I feel like I should also point out that there are differences between Microsoft’s implementation of StreamReader(string path) and my example that I think are important to note. My example will construct a new instance of a FileStream first and then pass it to the StreamReader’s constructor. The StreamReader(string path) constructor performs all of the appropriate argument checks and then constructs the FileStream inside of the constructor and then immediately passes that new instance to a method which stores the stream reference on the new StreamReader instance. If the StreamReader constructor happened to throw an exception due to arguments being invalid then that exception would occur prior to the FileStream getting constructed so there would be no danger of leaking the FileStream. This is a slightly different scenario then my example of calling the FileStream constructor as one of the constructor parameters to the StreamReader. Actually this scenario would probably make for a good example #6 in terms of constructing disposable objects inside of a constructor.

Leave a Comment

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: