Hidden IDisposable Issues in Microsoft .NET

Out of all the millions of lines of code I have reviewed in the past 20 years of .NET, hands down the #1 issue are developers not properly disposing of objects that implement IDisposable. This article shows the incorrect and then the correct way to handle some of these types such as StringReader.

As an experienced .NET software engineer, whenever I am hired into a new team, I almost immediately get tasked to fix coding issues. I guess since I am dotNetDave and I wrote the only coding standards book for .NET! Out of all the millions of lines of code I have reviewed in the past 20 years of .NET, hands down the #1 issue are developers not properly disposing of objects that implement IDisposable. In some teams, it has taken me many months of hard work to fix these issues that will always lead to virtual memory leaks.

While working on a solution recently, I Tweeted the following:

This post is to answer Evil Mikes question. If you have a coding question for me, please email me using this page: https://dotnettips.wordpress.com/about/.

The Problem

The problem I am seeing is that I see a lot of code like this:

public static TResult Deserialize<TResult>(string xml) where TResult : class
{
    var xs = new XmlSerializer(typeof(TResult));
    return (TResult)xs.Deserialize(new StringReader(xml));
}

The issue with this is the final line where a new StringReader is created. StringReader (and just about every type that ends in ‘Reader’) is one of those types that implement IDisposable to clean up the memory it uses. Since it does, it’s extremely important that the developer call .Dispose when it’s no longer needed or use the using code block.

I can prove that Dispose is not being called by looking at the IL for this method.

As you can see, there isn’t a call to Dispose in the IL.

The Solution

The following code, from my Spargine open-source project, properly codes this method as shown below:

public static TResult Deserialize<TResult>(string xml) where TResult : class
{
    using var sr = new StringReader(xml);
    var xs = new XmlSerializer(typeof(TResult));

    return (TResult)xs.Deserialize(sr);
}

As you can see, this uses the new way of implementing the using code statement. This means that at the end of the code block, in this case the method, Dispose will be called on StringReader, and its memory will be cleaned up. Want proof? Here it is:

As you can see, a try/finally is created by the compiler and in the finally Dispose is being called. This should be the only way to code types that implement IDisposable and not the way that was first shown.

Summary

Since these issues have been around ever since .NET was created, I wished Visual Studio would alert the coder that they are causing a memory leak! Until then, be careful or hire me to help your team!

If you have any comments or suggestions, please comment below.