What's the best way to avoid memory leaks in WPF PRISM/MVVM application

Question

I have a WPF application based on PRISM that utilizes the MVVM pattern.

I have noticed that occasionally my view models, views and everything connected to them will hang around long after their intended lifespan.

One leak involved subscribing to CollectionChanged on a collection belonging to an injected service, another involved not calling the Stop method on a DispatcherTimer, and yet another required a collection be cleared of it's items.

I feel using a CompositePresentationEvent is probably preferable to subscribing to CollectionChanged, but in the other scenarios I am leaning towards implementing IDisposable and have the views call the Dispose method on the view models.

But then something needs to tell the view when to call Dispose on the view model, which gets even less attractive when the complexity of the views increase, and they start including child views.

What do you think is the best approach to handling view models, to ensure they don't leak memory?

Thanks in advance

Ian

Answer 1

I can tell you that I've experienced 100% of the pain you have experienced. We are memory leak brothers, I think.

Unfortunately the only thing I've figured out to do here is something very similar to what you are thinking.

What we've done is create an attached property that a view can apply to itself to bind a handler to the ViewModel:

<UserControl ...
             common:LifecycleManagement.CloseHandler="{Binding CloseAction}">
...
</UserControl>

Then our ViewModel just has a method on it of type Action:

public MyVM : ViewModel
{
     public Action CloseAction
     {
          get { return CloseActionInternal; }
     }

     private void CloseActionInternal()
     {
          //TODO: stop timers, cleanup, etc;
     }
}

When my close method fires (we have a few ways to do this... it's a TabControl UI with "X" on the tab headers, that kind of thing), I simply check to see if this view has registered itself with the AttachedProperty. If so, I call the method referenced there.

It's a pretty roundabout way of simply checking to see if the DataContext of a View is an IDisposable, but it felt better at the time. The problem with checking the DataContext is you might have sub view models that also need this control. You'd either have to make sure your viewmodels chain forward this dispose call or check all of the views in the graph and see if their datacontexts are IDisposable (ugh).

I sort of feel like there is something missing here. There are a few other frameworks that attempt to mitigate this scenario in other ways. You might take a look at Caliburn. It has a system for handling this where a ViewModel is aware of all sub view models and this enables it to automatically chain things forward. In particular, there is an interface called ISupportCustomShutdown (I think that's what it's called) that helps mitigate this problem.

The best thing I've done, however, is make sure and use good memory leak tools like Redgate Memory Profiler that help you visualize the object graph and find the root object. If you were able to identify that DispatchTimer issue, I imagine you are already doing this.

Edit: I forgot one important thing. There is a potential memory leak caused by one of the event handlers in DelegateCommand. Here's a thread about it on Codeplex that explains it. http://compositewpf.codeplex.com/WorkItem/View.aspx?WorkItemId=4065

The latest version of the Prism (v2.1) has this fixed. (http://www.microsoft.com/downloads/details.aspx?FamilyID=387c7a59-b217-4318-ad1b-cbc2ea453f40&displaylang=en).

Answer 2

My findings so far...

In addition to PRISM, Unity, WPF and MVVM we are also using Entity Framework and the Xceed data grid. Memory profiling was done using dotTrace.

I ended up implementing IDisposable on a base class for my view models with the Dispose(bool) method being declared virtual allowing sub classes the chance to clean up as well. As each view model in our application gets a child container from Unity we dispose it as well, in our case this ensure that EF's ObjectContext went out of scope. This was our major source of memory leaks.

The view model is disposed within an explicit CloseView(UserControl) method on a base controller class. It looks for an IDisposable on the DataContext of the view and calls Dispose on it.

The Xceed data grid seems to be causing a fair share of the leaks, especially in long running views. Any view that refreshes the data grid's ItemSource by assiging a new collection should call Clear() on the existing collection before assigning the new one.

Be careful with Entity Framework and avoid any long running object contexts. It's very unforgiving when it comes to large collections, even though you have removed the collection if tracking is turned on, it will hold a reference to every item in the collection even though your no longer hanging on to them.

If you don't need to update the entity retrieve it with MergeOption.NoTracking, especially in long lived views that bind to collections.

Avoid views with a long life, don't hold onto them within a region when they are not visibile, this will cause you grief especially if they refresh their data at regular intervals when they are visible.

When using CellContentTemplates on the Xceed Column don't use dynamic resources as the resource will hold a reference to the cell, which in turn keep the entire view alive.

When using CellEditor on the Xceed Column and the resource is stored in an external resource dictionary add x:Shared="False" to the resource containing the CellEditor, once again the resource will hold a reference to the cell, using x:Shared="False" ensures you get a fresh copy each time, with the old one being removed correctly.

Be careful when binding the DelegateCommand to items within the Exceed data grid, if you have a case such as a delete button on the row which binds to a command, be sure to clear the collection containing the ItemsSource before closing the view. If you're refreshing the collection you also need to reinitialize the command as well as the command will hold a reference to each row.