In my role as a seasoned .NET software engineer, each time I join a new team, I find myself promptly assigned the task of resolving coding glitches. Perhaps it’s because of my reputation as dotNetDave and the authorship of the definitive coding standards book for .NET! With the vast expanse of code – spanning millions of lines – that I’ve reviewed over the past two decades in the .NET ecosystem, one critical problem consistently stands out: the inadequate disposal of objects implementing IDisposable. Within certain teams, I’ve invested several months of dedicated effort to rectify these issues, which inevitably culminate in virtual memory leaks.
During the course of tackling a recent solution, I shared the following message on Twitter:
This post is to answer Evil Mike’s question. For any coding inquiries you may have, please don’t hesitate to contact me via email by utilizing this page: https://dotnettips.wordpress.com/about/. Your questions are highly valued, and I’m here to provide assistance.
The Issue
What I’ve observed is a recurring concern involving the usage of disposable objects in code, often manifested as follows:
public static TResult Deserialize<TResult>(string xml) where TResult : class
{
var xs = new XmlSerializer(typeof(TResult));
return (TResult)xs.Deserialize(new StringReader(xml));
}
The notable concern arises in the final line, where a new StringReader instance is instantiated. StringReader, much like other types concluding with ‘Reader,’ falls within the category of objects that implement IDisposable for memory cleanup. Given this fact, it becomes paramount for developers to conscientiously invoke the .Dispose method once it serves its purpose or alternatively, to adopt the using code block for proper management.
I can substantiate the absence of Dispose being invoked by examining the Intermediate Language (IL) representation of this method.
As evident from the IL representation, no invocation to Dispose is present.
The Resolution
A viable solution lies within the subsequent code snippet, extracted from my open-source project Spargine. The method is appropriately structured as depicted below:
public static TResult Deserialize<TResult>(string xml) where TResult : class
{
using (var sr = new StringReader(xml))
{
using (var xmlReader = XmlReader.Create(sr))
{
return (TResult)new XmlSerializer(typeof(TResult)).Deserialize(xmlReader);
}
}
}
Notice that I’ve implemented the using code block for both StringReader and XmlReader. This judicious approach ensures that Dispose will be invoked, thereby facilitating memory cleanup upon the conclusion of the block. Need evidence? Allow me to present it:
Observe that the compiler generates a try/finally construct, wherein Dispose is invoked within the finally block. This method should be exclusively employed for coding types that implement IDisposable, standing in stark contrast to the initial approach showcased.
Summary
Given that these concerns have persisted since the inception of .NET, it’s my hope that Visual Studio could eventually provide a prompt to alert coders about potential memory leaks. Until such a feature materializes, exercising caution remains crucial. Alternatively, consider enlisting my assistance to fortify your team’s codebase.
Once more, I extend my gratitude to Evil Mike for posing the question—please continue to share your inquiries! Should you possess any comments or suggestions, kindly contribute them below.
Pick up any books by David McCarter by going to Amazon.com: http://bit.ly/RockYourCodeBooks
Make a one-time donation
Make a monthly donation
Make a yearly donation
Choose an amount
Or enter a custom amount
Your contribution is appreciated.
Your contribution is appreciated.
Your contribution is appreciated.
DonateDonate monthlyDonate yearlyIf you liked this article, please buy David a cup of Coffee by going here: https://www.buymeacoffee.com/dotnetdave
© The information in this article is copywritten and cannot be preproduced in any way without express permission from David McCarter.
dotNetTips.com 
IDisposable is probably the most abused interface in the entire eco system. Direct from the documentation: “Provides a mechanism for releasing unmanaged resources.” If you r class does not have a member which contains something that holds on to an unmanaged resource (directly or indirectly) your class has no business implementing IDisposable….
Consider the overhead (knowing about Finalizers, ensure Dispose can be called multiple times, accssing a member must throw ObjectDisposedException, any class that has a member of your type must also implement IDisposable (and take on all thos constraints.
Remember [Dave, I know you know this].. asside from the syntactic sugar of the using statement, the .Net runtime will NEVER implicitly call the Dispose method.
If you are not holding an unmanaged resource, do yourself (And every consumer of your code) a big favor and use one of the many other design patterns to manage operations as an item goes out of scope.
StringReader is not the best example of the case of “extremely important” disposing. Its Dispose does nothing.
Hi David,
I came across the below code block sometime back, not sure if using is really helping here –
YES, use using hear. StreamReader will cause a leak.
Over the years, I have experimented with a pattern I call MustDispose. It’s an object which throws an exception if it’s ever garbage collected without first being disposed. Here’s an old version.
// Copyright © 2003 by Martin L. Shoemaker
// All rights reserved.
using System;
using System.Diagnostics;
namespace Disposable
{
///
/// Include a member of class MustDispose in a class for which you
/// want to require that Dispose be called.
///
///
/// This object will either throw an exception or assert if it
/// is destroyed without being disposed first. If you include it in
/// your class, and call its Dispose in your Dispose, then your
/// class gets a “standard” implementation of mandatory disposal.
///
/// This object also serves as a flag for whether Dispose has been
/// called or not. IDisposable rules require that redundant calls
/// to Dispose are harmless.
///
/// Example usage:
///
/// MustDispose m_Dispose = new MustDispose( "MyClass" , true );
///
/// public void Dispose()
/// {
/// if ( ! m_Dispose.Disposed )
/// {
/// // TODO: Clean up resources.
///
/// // We cleaned up. Record that.
/// m_Dispose.Dispose();
/// }
/// }
///
///
public class MustDispose : IDisposable
{
private string m_ClassName;
private bool m_Throw = false;
private bool m_Disposed = false;
}
Is this something that can be picked up by static analysis tools? I imagine in a sprawling legacy codebase there could be hundreds of instances of code like this.
I have not found any tool that finds everything. That’s why it still comes down to checking manually to find everything. I will be revisiting this for Part 3.
Hi David,
While browsing through the reference implementation for StringReader.cs I noticed that the Dispose() method simply sets some class attributes to their default values, i.e. there’s no unmanaged resource releasing happening (see https://referencesource.microsoft.com/#mscorlib/system/io/stringreader.cs).
I think in your example there’s no potential for a memory leak because the StringReader doesn’t own unmanaged resources, and would be GCed anyways if it goes out of scope.
You are correct. I treat all types that implement IDisposabe as a potential issue. I don’t have time to inspect each type and its base types. So I recommend just coding any IDispoable type the same. Plus we have no idea if they will add things in the future that could cause virtual memory leaks.