New Fun Blog – Scott Bilas

Take what you want, and leave the rest (just like your salad bar).

Assertive Finalizers

with 5 comments

Oruro

In my previous post, I talked about why I have stopped using finalizers for unmanaged resource collection. I want this to be done through the disposable pattern instead, forcing the programmer to manage resources manually.

Ironically, finalizers are a great way to verify this.

Sander van Rossen quickly figured out where I was going with this and proposed in a comment that we can just assert in a finalizer. We just need a couple more things:

  1. We need to track the source of the object to figure out where the leak is.
  2. We need to ensure that finalizers run on shutdown, or our assert will never get hit.

This is familiar territory to C++ programmers. Most of us use memory management libraries that provide leak detection and reporting. Let’s do something similar in C#.

DisposableBase

If you look around online, you’ll find some full-featured IDisposable base classes intended to deal with the full finalization model. We can eliminate most of that as either too complicated or unnecessary. We just need a few things:

  • A stack trace grabbed at construction time
    This will be used for the error report. Because this is expensive to gather, we need to control it with an #if that is off by default, only turned on if needed to help an investigation. Most of the time a leaked disposable will be easy to find by inspection, but in the 5% case we will need a lot more context.
  • A finalizer that reports the problem
    It could throw an exception, fire an assert, or route to an error reporter. Depends on the application. In my example I just have it output to the debug window for a demo.
  • Disposal helper methods
    These wrap up disposal a bit, so inheritors only need to implement OnDispose. The most important feature, though, is that Dispose will call GC.SuppressFinalize when our object is disposed. This eliminates the performance cost of having a finalizer in the normal case when clients are disposing this class properly. This is why the finalizer has no “if” in it – if it ever gets called, then we have a bug.

This is what I am currently using as my base class for handling unmanaged resources:

[code lang=”csharp”]
// comment out unless diagnosing a leak
#define DEBUG_DISPOSE

public abstract class DisposableBase : IDisposable
{
// store stack at point of construction for possible later use
# if DEBUG_DISPOSE
StackTrace _trace = new StackTrace(true);
# endif

// finalizer will not be called if object was properly disposed
~DisposableBase()
{
string message = "!! Forgot to dispose a " + GetType().FullName;
# if DEBUG_DISPOSE
message += "\n\nStack at construction:\n\n" + _trace + "!!";
# endif
Debug.WriteLine(message);
}

public bool IsDisposed { get; private set; }

public void Dispose()
{
ThrowIfDisposed();
IsDisposed = true;

try { OnDispose(); }
finally { GC.SuppressFinalize(this); }
}

protected abstract void OnDispose();

protected void ThrowIfDisposed()
{
if (IsDisposed)
throw new ObjectDisposedException(GetType().FullName);
}
}
[/code]

Demonstration

Here is a simple test app that shows what happens if we forget to dispose an instance.

[code lang=”csharp”]
public class DatabaseConnection : DisposableBase
{
protected override void OnDispose()
{ Debug.WriteLine("Disposing"); }
}

public class Program
{
static void Main(string[] args)
{
using (var remembered0 = new DatabaseConnection())
using (var remembered1 = new DatabaseConnection())
{
}

var forgotten = new DatabaseConnection();

GC.Collect();
GC.WaitForPendingFinalizers();
}
}
[/code]

The first two instances dispose fine, but the third is leaked and so we get a log to the output window:

[code lang=”text”]
Disposing
Disposing
!! Forgot to dispose a DatabaseConnection

Stack at construction:

at DisposableBase..ctor() in C:\Users\Scott\Cloud\Proj\tests\BlogSamples\ConsoleApplication1\Finalizers.cs:line 18
at DatabaseConnection..ctor()
at Program.Main(String[] args) in C:\Users\Scott\Cloud\Proj\tests\BlogSamples\ConsoleApplication1\Finalizers.cs:line 89
at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean ignoreSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ThreadHelper.ThreadStart()
!!
[/code]

We can easily zero in on the exact spot where the leaked resource was allocated and wrap it in a ‘using’ to resolve.

Note the use of GC.Collect and GC.WaitForPendingFinalizers right before the test application exits. This is necessary in order to force all finalizing objects to be collected and reported before process shutdown, otherwise they are simply dropped by the system when the process’s memory is released.

These two can even be called during normal app run as well for leak testing without needing to wait for shutdown. This would be useful in a live service with a periodic leak for which we want an in-session nonfatal log of leaks.

What About External Classes?

This takes care of our own classes, where we have full control. But what about system or third party classes that are using finalizers? With those it’s back to square one.

Well, I suppose would write a helper class..

[code lang=”csharp”]
public class SafeDisposer<T> : DisposableBase where T : IDisposable
{
public SafeDisposer(T disposable) { Obj = disposable; }

public T Obj { get; private set; }

protected override void OnDispose()
{
Obj.Dispose();
Obj = default(T);
}
}

public static class SafeDisposer
{
public static SafeDisposer<T> Wrap<T>(T disposable) where T : IDisposable
{ return new SafeDisposer<T>(disposable); }
}
[/code]

And equivalent updates in the demo code:

[code lang=”csharp”]
public class Program
{
static void Main(string[] args)
{
using (var remembered2 = SafeDisposer.Wrap(new StringReader("foo")))
using (var remembered3 = SafeDisposer.Wrap(new StringReader("poo")))
{
remembered2.Obj.Peek();
}

var forgotten2 = SafeDisposer.Wrap(new StringReader("boo"));

GC.Collect();
GC.WaitForPendingFinalizers();
}
}
[/code]

And output:

[code lang=”text”]
!! Forgot to dispose a SafeDisposer`1[[System.IO.StringReader, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]]

Stack at construction:

at DisposableBase..ctor() in C:\Users\Scott\Cloud\Proj\tests\BlogSamples\ConsoleApplication1\Finalizers.cs:line 18
at SafeDisposer`1..ctor(T disposable) in C:\Users\Scott\Cloud\Proj\tests\BlogSamples\ConsoleApplication1\Finalizers.cs:line 53
at SafeDisposer.Wrap[T](T disposable) in C:\Users\Scott\Cloud\Proj\tests\BlogSamples\ConsoleApplication1\Finalizers.cs:line 67
at Program.Main(String[] args) in C:\Users\Scott\Cloud\Proj\tests\BlogSamples\ConsoleApplication1\Finalizers.cs:line 98
at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean ignoreSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ThreadHelper.ThreadStart()
!!
[/code]

So to use, we just wrap a disposable at the point of creation in SafeDisposer.Wrap() and access it through “.Obj”. Not bad, but not great either. I’m ok with the wrapper function, but the required access via the Obj member is a pain, and also means that converting an unsafe disposable into a safe one requires updating a lot of code.

Another option is (maybe) to use a dynamic proxy system to inject the functionality we need, letting client code remain unchanged except at the point of creation. Or perhaps a run-time system that patches system assemblies to do the injection.

I’ll leave this as an exercise for the reader because I think we’re probably getting seriously diminishing returns at this point. Most disposable objects in a large system will be classes that we have full control over. The few objects not under our control will likely be low level primitives that will be wrapped up by our own foundation classes anyway.

March 8th, 2010 at 6:04 pm

Posted in .net,programming

5 Responses to 'Assertive Finalizers'

Subscribe to comments with RSS or TrackBack to 'Assertive Finalizers'.

  1. […] I say “rich base class”. First is for mixing in object functionality. I’ve spoken about this before. There really is no way to do this nicely in C#. However, today’s post is about the other thing […]

  2. Hi, I’m using a similar approach. Note that, as per the MSDN documentation, GC.WaitForPendingFinalizers is NOT guaranteed to exit, ever. I’ve seen it block and had to accommodate for this behavior with something like the following: house GC.WaitForPendingFinalizers in a try-catch(ThreadAbortException) in a temporary new Thread. Start, Join(w/timeout), Abort.

    At this point, it may make sense to house this functionality so it can be called with one line of code from, say, a static method like:
    static void SafeDisposer.ValidateDisposals(timeout) {…}

    David

    29 Nov 10 at 7:32 pm

  3. WaitForPendingFinalizers won’t exit if objects keep resurrecting themselves or doing other weird things in their finalizers. To me that signifies a design flaw in the code. Should be ultra rare and in fact I’ve never seen it happen myself.

    I suppose if I did see it a few times I’d do the start/join/abort you mention, and have the code file a minidump with the bugbase before the abort as a high severity issue.

    Scott

    29 Nov 10 at 9:38 pm

  4. To my knowledge there are a number of other ways in which WaitForPendingFinalizers will block other than constant object resurrections keeping the finalizer queue from getting empty (I’ve never seen anyone try to resurrect their object through the finalizer… that would be pretty strange/evil/scary). For instance:
    * The finalizer thread blocks because any finalizer it is running blocks.
    * The finalizer thread blocks because it needs to marshal to the UI thread and:
    ** The UI thread is no longer pumping messages (IE at program shutdown time which is precicely when we’re calling it with this pattern – some COM objects may cause this).
    ** The UI thread is also WaitForPendingFinalizers… or some other deadlock/blocking condition.

    But all of the above is irrelevant. The fact is, the documentation says WaitForPendingFinalizers is not guaranteed to exit, so we can’t rely on it to exit. Lets say some Windows Update or more likely a new .NET version comes along and changes the behavior a little bit but still abides by the documentated behaviors… just look at how many people changes like the .NET 1.1 to .NET 2.0 changes broke because they were relying on undocumented behaviors of GC.Collect and WaitForPendingFinalizers. Relying on undocumented behaviors (even “good” ones such as situations where it _didn’t_ used to block) means that such updates are free to break our code w/out apologies.

    That said, the simplicity of the two-line approach you use is certainly worth taking a reactive bugfix approach to, so long as the tool is internal or whatnot. I wouldn’t have implemented the Thread approach if I hadn’t seen the problem firsthand.

    David

    30 Nov 10 at 1:54 pm

  5. All very good points, thanks for your reply.

    As I mainly do internal tools, the reactive bugfix approach has always been fine. Though I’d say the same for even production tools – this article is about reporting leaks, which is a debug feature, so we’d not want it to compile in release builds anyway. I’d just wrap the GC calls in #if DEBUG and call it good. :)

    Scott

    30 Nov 10 at 8:43 pm

Leave a Reply