10

Closed

Memory Leak caused by DelegateCommand.CanExecuteChanged Event

description

When profiling my application I noticed that plenty of EventHandlers had never been deregistered from DelegateCommand's CanExecuteChanged-Event. So those EventHandlers were never been garbage-collector, which caused a severe memory leak.
 
As registering CanExecuteChanged-EventHandles is done outside application code scope I had expected them to be deregistered automatically as well. At this point I thought this might as well be a ThirdParty WPF control issue, but digging further I read a blog post stating that "WPF expects the ICommand.CanExecuteChanged-Event to apply WeakReferences for EventHandlers". I had a look into RoutedCommand, and noticed it uses WeakReferences as well.
 
I adapted DelegateCommand to use an implementation similar to RoutedCommand's CanExecuteChanged-Event, and the memory leak was gone. The same is true for CompositeCommand.

comments

blainew wrote Jun 15, 2009 at 5:03 PM

The Prism team is looking at this issue.

Blaine Wastell

mcrimes wrote Jun 16, 2009 at 1:41 PM

Our application regularly creates new views/ViewModels and disposes of them & this bug has quite a large knock on effect. We've worked around for now by explicitly releasing all the DelegateCommands & their events upon disposal, but I'd argue that its Impact level of Low is not really realistic & it should be upgraded to a higher level.

andersonimes wrote Jul 17, 2009 at 4:54 PM

This was causing us a pretty bad memory leak (Views and associated ViewModels hanging around. I had to edit this to include WeakReference support today.

ifioravanti wrote Oct 8, 2009 at 10:07 PM

Any news on the status of this issue ? andersonimes can you please share your fix ?

ifioravanti wrote Oct 9, 2009 at 7:17 PM

I saw that suggested patch has been implemented in V3 SourceCode. We'll give it a try.

andersonimes wrote Oct 9, 2009 at 8:02 PM

ifioravanti: thanks for saying this... I was in the middle of posting my solution... now I don't have to. :)

andersonimes wrote Oct 9, 2009 at 8:08 PM

Just tried this implementation and verified no leaks. Thanks Prism folk!