Ayende @ Rahien

It's a girl

Sometimes it looks like select IS broken: A WPF memory leak

One of the most annoying bug reports that we got for NH Prof is that it crash with out of memory exception after long use. We did the usual checkups, and the reason for the memory leak was obvious, something kept a lot of objects internal to WPF in memory. In fact, here are the heavy hitters, as extracted from the dump:

Count Size (kliobytes) Type
5,844 3,337 System.Byte[]
69,346 5,474 System.String
49,400 37,198 System.Object[]
1,524,355 47,636 MS.Utility.SingleItemList`1[[System.WeakReference,mscorlib]]
3,047,755 71,432 MS.Internal.Data.ValueChangedEventArgs
1,523,918 71,434 MS.Utility.ThreeItemList`1[[System.WeakReference,mscorlib]]
3,048,292 71,444 MS.Utility.FrugalObjectList`1[[System.WeakReference,mscorlib]]
3,048,292 95,259 System.Windows.WeakEventManager+ListenerList
3,047,755 166,674 MS.Internal.Data.ValueChangedEventManager+ValueChangedRecord
3,056,462 191,029 System.EventHandler
7,644,217 238,882 System.WeakReference

As you can see, this just says that we are doing something that cause WPF to keep a lot of data in memory. In fact, this looks like a classic case of “memory leak” in .NET, where we aren’t releasing references to something. This usually happen with events, and it was the first thing that I checked.

It took a while, but I convinced myself that this wasn’t that. The next step was to try to figure out what is causing this. I’ll skip the sordid tale for now, I’ll queue it up for posting at a later date. What we ended up with is a single line of code that we could prove caused the issue. If we removed it, there was no leak, if it was there, the leak appeared.

That was the point where I threw up my hands and asked Christopher to look at this, I couldn’t think of something bad that we were doing wrong, but Christopher and Rob are the experts in all things WPF.

Christopher managed to reproduce this in an isolated fashion. here is how it goes:

public class TestModel : INotifyPropertyChanged
{
private readonly DispatcherTimer timer;
private int count;

public event PropertyChangedEventHandler PropertyChanged = delegate { };

public TestModel()
{
timer = new DispatcherTimer(DispatcherPriority.Normal)
{
Interval = TimeSpan.FromMilliseconds(50)
};
timer.Tick += Timer_Tick;
timer.Start();
}

public IEnumerable Data
{
get
{
return new[]
{
new {Name = "Ayende"},
};
}
}

private void Timer_Tick(object sender, EventArgs e)
{
if (count++ % 100 == 0)
{
GC.Collect(2, GCCollectionMode.Forced);
Console.WriteLine("{0:#,#}", Process.GetCurrentProcess().WorkingSet64);
}
PropertyChanged(this, new PropertyChangedEventArgs("Data"));
}
}

This is a pretty standard model, showing data that updates frequently. (The short time on the time is to show the problem in a short amount of time.) Note that we are being explicit about forcing a GC release here, to make sure it isn’t just waste memory that haven’t been reclaimed yet.

And the XAML:

<Grid>
<ItemsControl ItemsSource="{Binding Data}">
<ItemsControl.ItemTemplate>
<DataTemplate>
<StackPanel Orientation="Horizontal">
<TextBlock Text="{Binding Name}" />
</StackPanel>
</DataTemplate>
</ItemsControl.ItemTemplate>
</ItemsControl>
</Grid>

Executing this will result in the following memory pattern:

image

Looking at the code, I really don’t see anything that is done wrong there.

I uploaded a sample project that demonstrate the issue here.

Am I going crazy? Am I being stupid? Or is select really broken?

Comments

Lee Culver
10/01/2009 12:14 AM by
Lee Culver

Interesting. What exact version of CLR are you running? (If you attach with a native debugger, like windbg, and use "lmvm mscorwks", what's the output?)

How long did the process run for? (I assume the numbers at the bottom are minutes?)

Erik
10/01/2009 12:41 AM by
Erik

You said "This usually happen with events, and it was the first thing that I checked." -- How? Do you know of any good references talking about this issue?

Ayende Rahien
10/01/2009 12:43 AM by
Ayende Rahien

The output is in 50 seconds, actually, not in minutes. It run for about 5 minutes or so, I think.

I am using .NET 3.5 SP1

Here is output from windbg

0:017> lmvm mscorwks

start end module name

000007fef2650000 000007fef2ffe000 mscorwks (deferred)

Image path: C:\Windows\Microsoft.NET\Framework64\v2.0.50727\mscorwks.dll

Image name: mscorwks.dll

Timestamp:        Thu Jun 04 06:58:39 2009 (4A27466F)

CheckSum:         0099D011

ImageSize:        009AE000

File version:     2.0.50727.4927

Product version:  2.0.50727.4927

File flags:       0 (Mask 3F)

File OS:          4 Unknown Win32

File type:        2.0 Dll

File date:        00000000.00000000

Translations:     0409.04b0

CompanyName:      Microsoft Corporation

ProductName:      Microsoft® .NET Framework

InternalName:     mscorwks.dll

OriginalFilename: mscorwks.dll

ProductVersion:   2.0.50727.4927

FileVersion:      2.0.50727.4927 (NetFXspW7.050727-4900)

FileDescription:  Microsoft .NET Runtime Common Language Runtime - WorkStation

LegalCopyright:   © Microsoft Corporation.  All rights reserved.

Comments:         Flavor=Retail
Tobin
10/01/2009 12:43 AM by
Tobin

To ask the n00b question - what do you mean by "select"? I'm looking for a Linq Select statement or something, do you just mean the binding?

Ayende Rahien
10/01/2009 12:44 AM by
Ayende Rahien

Erik,

Manual code review, checking gc roots, etc.

Look for debugging memory leaks, you'll see a lot of articles about how to figure that out.

Ayende Rahien
10/01/2009 12:45 AM by
Ayende Rahien

Tobin,

It is a reference to The Pragmatic Programmer: select() isn't broken.

Tobin
10/01/2009 12:49 AM by
Tobin

Ah, got you now, damn in-jokes :)

It looks like the binding isn't releasing the old values when the OnPropertyChanged is being fired and refreshing the data?

Ayende Rahien
10/01/2009 12:52 AM by
Ayende Rahien

Yeah, I can't figure out why this is happening, though.

Will Hughes
10/01/2009 12:55 AM by
Will Hughes

Very interesting

We're getting this EXACT same issue with a different app but using the same bits you're talking about.

WPF, Timers, Event Handlers, and INotifyPropertyChanged.

We also have a bunch of Observable collections of types that implement INotifyPropertyChanged.

I had started digging down, but hadn't gotten further than getting the memory dump and doing some preliminary poking around the codebase.

I figured it was us doing something wrong. Good to see others having the same issue.

Looking forward to seeing the resolution to this though.

Also - Tobin, the reference about "select being broken" is from a fairly common bit of lore that goes along the lines of "If you think SELECT is broken, then you're doing something wrong".

i.e If you think something fundamental and well used by everyone, then there's a pretty damn good chance that you're using it wrong.

Will Hughes
10/01/2009 12:56 AM by
Will Hughes

Oh, right, The Pragmatic Programmer. Duh. sigh

Jacob
10/01/2009 12:58 AM by
Jacob

My guess is you're running into this issue: http://support.microsoft.com/kb/938416

But changing {Binding Name} to {Binding Name, Mode=OneTime} doesn't appear to resolve it. (It definitely slows it down, though.)

Tobin
10/01/2009 01:08 AM by
Tobin

The thing is, you see quite a few WPF programs slowly go up in memory - so it could be a common thing. That or it's an easy thing to blame it on.

I remember Witty always slowly took more memory, even if I cleared the tweets in it.

Nick Aceves
10/01/2009 01:50 AM by
Nick Aceves

What commands did you use to extract that information from the dump? I'm also debugging a CLR memory leak, and info about what kind of objects are taking up memory would be incredibly helpful

Steve Py
10/01/2009 02:46 AM by
Steve Py

During the test, checks assert that PropertyChanged only has one listener registered? (not getting caught up registering a new listener, or maybe something with how it's hooked up?)

Kurt Harriger
10/01/2009 02:55 AM by
Kurt Harriger

I'm not a wpf programmer, but from what I have seen wpf uses a lot of static variables and the weakreference seems to indicate that wpf might be storing weakreferences in a static variable somewhere. Although the target of the weak reference is collected the weakreference object will hand around in some static list somewhere where some background thread will periodically remove weakreferences that no longer point to a valid object. But, in your case probably not nearly often enough. Your job if you choose to accept it is to find out where and if you can force a cleanup..

The Weakeventmanager class looks promising, but purge does not appear to be public or static. Digging around in reflector a bit it looks like there is a singleton weakeventtable that is cleaned up when idle. the class is internal (of course), but you could try calling WeakEventTable.CurrentWeakEventTable.Cleanup through private reflection. If that works then maybe more digging will reveal some publicly accessible method that could force a cleanup. I could be completely wrong on this but worth a try.

ct
10/01/2009 03:07 AM by
ct

If I minimize the WPF app the memory goes down just fine. Restoring and it grows.

So something about minimizing causes it to release the memory. Sounds like it might be something MS is holding onto internally for drawing isn't cleared unless it has nothing to draw (ie. when minimized perhaps?)

Rafal
10/01/2009 05:05 AM by
Rafal

I think you are guilty of thinking that automated tests and paying attention to infractructure design lead to error free applications. Now you have been reprimanded for forgetting about Microsoft's hard work.

Nid
10/01/2009 05:51 AM by
Nid

I remember colleagues talking about a problem when bindind real time data to a TextBox. The undo feature was accumulating all previous values, even if the TextBox is in readonly mode. They had to disable undo on the textbox to fix the leak.

Mr. Daisy
10/01/2009 05:58 AM by
Mr. Daisy

I strongly believe if some WPF core component was broken, VS10 in WPF wouldn't be possible.

As we can see, it just is, so it is safe to presume you must be doing something wrong.

Have you tried MSDN forums?

Gian Maria
10/01/2009 06:16 AM by
Gian Maria

Tracing with Dot Trace it is obvious that WeakReference object will never gets disposed, because their number grow continuosly.

The problem maybe is due to the fact that the view model exposes data property ad simple property, and not as a dependencyproperty, as ususal.

I did a very quick try, but if you make your model inherit from dependenyobject, transform the data property in a DependencyProperty all those weak references seems to be gone away.

Alk.

alwin
10/01/2009 06:17 AM by
alwin

Does the memory go up when you keep returning the same object from the Data property?

BTW have you asked this on stack overflow? Never know if someone knows the answer there.

alwin
10/01/2009 06:27 AM by
alwin

Oh and if you return a 'real' (I mean named) class from Data, you can more easily debug the heap to find instances of that class and references to it.

public IEnumerable Data {

get {

return new DataList() { new DataItem {Name = "Ayende"}; 

}

}

Then try to find references to DataList and DataItem with dumpheap or something, not sure how to do this though.

Maybe this can help:

blogs.microsoft.co.il/.../...tancesof-lt-t-gt.aspx
blogs.microsoft.co.il/.../...emo-2-and-demo-3.aspx

Steve Wagner
10/01/2009 06:47 AM by
Steve Wagner

The problem is that you return an array of objects that are not implement INotifyPropertyChanged and then Databind to property's of this object.

You can read more about here: support.microsoft.com/

Ayende Rahien
10/01/2009 09:08 AM by
Ayende Rahien

ct,

You'll see the same behavior in any Windows app. Windows reduce the working set of an app when it is minimized

Ayende Rahien
10/01/2009 09:10 AM by
Ayende Rahien

alwin,

Yes, the problem persist for named classes as well.

And there aren't live instances of the data class in memory, only the weak ref stuff

Ryan Heath
10/01/2009 10:23 AM by
Ryan Heath

Interesting, changing the code to:

public class MyModel : INotifyPropertyChanged

{

public event PropertyChangedEventHandler PropertyChanged = delegate { };

public string Name { get; set; }

}

...

new MyModel{Name = "Ayende"},

...

Seems to keep the mem usage low ...

// Ryan

Steve Wagner
10/01/2009 11:49 AM by
Steve Wagner

@Ryan Heath: Yep then this is the issue it talked about above.

Magnus Lund
10/01/2009 12:07 PM by
Magnus Lund

Could it be the Templates not being recycled that is the problem?

In SP1 they added the VirtualizingStackPanel.VirtualizationMode="Recycling" which is set to "Standard" default.

I have no idea, why it would hang on to the old templates, tho.

Andrew
10/01/2009 04:27 PM by
Andrew

I had similar problems with some 1.1 code that was upgraded to 3.5. In 1.1, Garbage collection worked fine for a particular scenario, but when it was upgraded to 3.5 it seemed that the Application, while not throwing any sort of exception, was slowly using more and more memory.

Change the code to read:

private void Timer_Tick(object sender, EventArgs e)

{

if (count++ % 100 == 0)

{

    TestModel.MinimizeMemory();

    //GC.Collect(2, GCCollectionMode.Forced);

    Console.WriteLine("{0:#,#}", Process.GetCurrentProcess().WorkingSet64);

}

PropertyChanged(this, new PropertyChangedEventArgs("Data"));

}

public static void MinimizeMemory()

{

GC.Collect(GC.MaxGeneration);

GC.WaitForPendingFinalizers();

SetProcessWorkingSetSize(Process.GetCurrentProcess().Handle, (UIntPtr)0xFFFFFFFF, (UIntPtr)0xFFFFFFFF);


IntPtr heap = GetProcessHeap();


if (HeapLock(heap))

{

    try

    {

        if (HeapCompact(heap, 0) == 0)

        {

            // error condition ignored            

        }

    }

    finally

    {

        HeapUnlock(heap);

    }

}

}

[DllImport("kernel32.dll")]

[return: MarshalAs(UnmanagedType.Bool)]

internal static extern bool SetProcessWorkingSetSize(IntPtr process, UIntPtr minimumWorkingSetSize, UIntPtr maximumWorkingSetSize);

[DllImport("kernel32.dll", SetLastError = true)]

internal static extern IntPtr GetProcessHeap();

[DllImport("kernel32.dll")]

[return: MarshalAs(UnmanagedType.Bool)]

internal static extern bool HeapLock(IntPtr heap);

[DllImport("kernel32.dll")]

internal static extern uint HeapCompact(IntPtr heap, uint flags);

[DllImport("kernel32.dll")]

[return: MarshalAs(UnmanagedType.Bool)]

internal static extern bool HeapUnlock(IntPtr heap);

Will"fix" the problem in the sense that it will stop the memory leak. It doesn't do a great job explaining it, unless the GC in 1.1 used to compact the process heap and 3.5 doesn't.

Mike Brown
10/01/2009 06:19 PM by
Mike Brown

The problem is that the weak references do not get unrooted. The trade off is that instead of rooting a FrameworkElement (which would kill your memory a lot quicker) you're rooting a WeakEvent object which despite having a much smaller memory footprint...still has one.

It appears that the PropertyChangedEventManager has a function that allows a WeakEventListener to be deregistered...but it doesn't get called by the framework.

Mike Brown
10/01/2009 08:06 PM by
Mike Brown

Digging in I discovered that the Binding System doesn't deregister the listener for the "Name" property when you notify that Data has changed.

It's definitely a bug and it has to do with the way the binding system looks at the binding in question. When you notify that Data has changed, rather than deregistering the existing bindings further down the tree (in this case the Name binding on your Textblock) and reusing the elements it appears that an entirely new set of elements are created. Unfortunately, the Textblock never gets the opportunity to deregister its binding.

Now if you made Data an observable collection (necessitates making your anonymous type into a full class) and made the collection raise a CollectionChanged event (e.g. Data[0]=Data[0]), everything works just fine.

Daniel
10/04/2009 12:38 PM by
Daniel

This is not the "by design" scenario described in KB 938416.

The condition "Object X contains a direct reference or an indirect reference to the target of the data-binding operation." is not fulfilled in this example.

This looks like a .NET Bug to me. I've tried .NET 4.0 Beta 1 and .NET 4.0 Sept. LCTP; this memory leak is still present in both of them.

Did anyone submit a bug report to Microsoft?

Ayende Rahien
10/04/2009 08:38 PM by
Ayende Rahien

Daniel,

Yes, the WPF team is aware of this issue.

Comments have been closed on this topic.