ICommand binding causing UI memory leak in WPF application

Question

I'm building a WPF app that connects to a SQL Server database using LINQ to SQL.

The main window of the app contains a ListView containing a series of detail views. The ItemSource of the ListView is bound to a collection of detail view model objects exposed as a property on the root view model. Each detail view model object composes several ICommand properties as well as a property exposing a detail model object, which in turn exposes the various data fields shown in the UI.

Analysis with the ANTS memory profiler shows that the objects being leaked are those contained in the detail model object, and some UI classes to which they are bound. Instances of these objects from previous refreshes are not being garbage collected.

ANTS has a tool that allows the user to trace chains of reference to identify why unwanted memory is being retained. When I use it, I find that all of the chains that show up have an ICommand in them. Accordingly, I've removed the offending ICommand, and found that the memory leak disappears.

Unfortunately, I need the ICommand to implement some important functionality. What is really confusing me is how it has a reference to the detail model object in the first place- they are two completely separate instance variables in the detail view model object.

Here is the constructor of the detail view model object (The reference to the RootViewModel is used for callbacks in some of the methods connected to the ICommands. I originally suspected that this might be causing a circular chain of references that might be the cause of the problem, but removing it doesn't seem to have any effect.)

public CarDataViewModel(CarData carDataItem, RootViewModel parentViewModel)
    {

        _parentViewModel = parentViewModel;
        CarDataModel = carDataItem;
        CompetingCheckboxStatus = CarDataModel.CurrentCar.Competing;
        AcknowledgeAlarm = new ParameterlessCommand(AcknowledgeAlarmClicked);
        Acknowledge = new ParameterlessCommand(AcknowledgeClicked);
        ShowReport = new ParameterlessCommand(ShowReportClicked);
        Cancel = new ParameterlessCommand(CancelClicked);
    }

Here's the xaml where the bindings are set up - AcknowledgeAlarm is the ICommand, CarDataModel is the detail model object:

<ListView x:Name="itemGridView"Grid.Row="1"ScrollViewer.HorizontalScrollBarVisibility="Disabled" ItemsSource="{Binding CarDataViewModels}" IsSynchronizedWithCurrentItem="True" Margin="0,0,0,0">
        <ListView.ItemTemplate>
            <DataTemplate>
                </DataTemplate.Resources>
                <Button Command="{Binding AcknowledgeAlarm}">
                    <Border DataContext="{Binding CarDataModel}" BorderBrush="{StaticResource GrayFadeBrush}" Background="White" BorderThickness="5">
                        <Grid> . . .

Answer 1

The CanExecuteChanged event handler is likely implicated in the leak.

WPF expects ICommand implementations to use weak references to the event handlers. You're using a normal .NET event which uses strong references, which can cause this leak.

The way you are creating the ParameterlessCommand instance seems to imply that CanExecute will always be true, and you don't need the event at all. Are you actually firing the event anywhere, or is OnCanExecuteChanged unused code?

If not, replace the event definition with:

public event EventHandler CanExecuteChanged { add {} remove {} }

This way the event does not store any handlers, and the view model avoids having a strong reference to the UI elements.

If you need to raise the event, the easiest solution is to use CommandManager.RequerySuggested, which matches the weak event semantics expected for ICommand:

public event EventHandler CanExecuteChanged {
    add {
        CommandManager.RequerySuggested += value;
    }
    remove {
        CommandManager.RequerySuggested -= value;
    }
}

---

Another thing you should do is implement INotifyPropertyChanged in your view model (if you haven't done so already), and use that instead of having individual NameChanged etc. events for each property. This is because the logic in WPF dealing with the individual properties causes memory leaks when there is a reference from the view model back to the UI elements: http://support.microsoft.com/kb/938416

AFAIK you need to implement INotifyPropertyChanged even if you don't actually have any change events.

---

My guess is that fixing either of these two problems will make the leak disappear: the incorrectly implemented CanExecuteChanged causes a strong reference from view model to view, which is exactly the circumstance under which the lack of INotifyPropertyChanged causes a leak.

But it's a good idea to fix both issues; not just one of them.