Beware of local scoping in .NET

Last week Jordan Parker and I were looking at a chunk of code that was evidently leaking memory.  What we discovered wasn't obvious at first so I thought I'd share it with the world.  The code looked something like this:

 
.cf1 { color: rgb(0,0,255) }
.bg1 { background: rgb(0,0,255) }
.cf2 { color: rgb(43,145,175) }
.bg2 { background: rgb(43,145,175) }
    void BackgroundThreadProc()
    {
        while (true)
        {
            _waitEvent.WaitOne();

            while (!WorkQueueEmpty())
            {
                WorkItem item = DequeueWorkItem();
                ProcessWorkItem(item);
            }
        }
    }

The idea is that we have a thread that waits to get signaled then flushes a queue of work items and goes back to waiting.  What could go wrong?  Imagine the following sequence of events:

1. a work item is queued and the background thread is signaled.
2. the background thread processes the work item and goes back to waiting for a signal.
3. a long time elapses before another work item is queued.

The problem we ran into is that after 2. the local variable "item" was still in scope.  From the C# perspective, you may think that "item" is out of scope while we're blocked in "WaitOne", however this is merely a language notion.  Local variables in .NET are scoped to methods, which means "item" is a GC root whenever we're executing this method.  This caused the most recently processes WorkItem not to be garbage collected, and that caused anything the WorkItem referenced not to be collected and so on.

We can see this by looking at the IL:

.method private hidebysig instance void BackgroundThreadProc() cil managed

{

.maxstack 2

.locals init (

[0] class WorkerThingy/WorkItem item)

L_0000: ldarg.0

L_0001: ldfld class [mscorlib]System.Threading.AutoResetEvent WorkerThingy::_waitEvent

L_0006: callvirt instance bool [mscorlib]System.Threading.WaitHandle::WaitOne()

L_000b: pop

L_000c: br.s L_001c

L_000e: ldarg.0

L_000f: call instance class WorkerThingy/WorkItem WorkerThingy::DequeueWorkItem()

L_0014: stloc.0

L_0015: ldarg.0

L_0016: ldloc.0

L_0017: call instance void WorkerThingy::ProcessWorkItem(class WorkerThingy/WorkItem)

L_001c: ldarg.0

L_001d: call instance bool WorkerThingy::WorkQueueEmpty()

L_0022: brfalse.s L_000e

L_0024: br.s L_0000

}

Notice that IL declares the locals near the top of the method body.  As far as the CLR is concerned 'item' IS in scope while we're waiting to get signaled.  This is something to keep in mind if you find yourself writing a method that never exits.