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:
- a work item is queued and the background thread is signaled.
- the background thread processes the work item and goes back to waiting for a signal.
- 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.
Comments
- Anonymous
October 15, 2007
PingBack from http://www.artofbam.com/wordpress/?p=8745 - Anonymous
September 20, 2010
An example of how to fix this would have been nice! - Anonymous
February 22, 2011
Thanks, I love language semantic stuff. helps us avoid these issues =)