Possible memory leak with WPF Custom Control?

Question

I might have a potential memory leak with my custom control. Do I actually have one?

public interface IAlertable : INotifyPropertyChanged { ... }

public sealed class AlertButton : Button
{
    private static readonly DependencyPropertyKey HasAlertPropertyKey = DependencyProperty.RegisterReadOnly("HasAlert", typeof(bool), typeof(AlertButton),
        new FrameworkPropertyMetadata(false, FrameworkPropertyMetadataOptions.None, null));

    public static readonly DependencyProperty HasAlertProperty = HasAlertPropertyKey.DependencyProperty;

    public static readonly DependencyProperty AlertContextProperty = DependencyProperty.Register("AlertContext", typeof(IAlertable), typeof(AlertButton),
        new FrameworkPropertyMetadata(null, FrameworkPropertyMetadataOptions.None, OnAlertContextChanged, null, false, UpdateSourceTrigger.PropertyChanged));

    static AlertButton()
    {
        DefaultStyleKeyProperty.OverrideMetadata(typeof(AlertButton), new FrameworkPropertyMetadata(typeof(AlertButton)));
    }

    private PropertyChangedEventHandler PropertyChangedEventHandler { get; set; }

    public bool HasAlert
    {
        get { return (bool)GetValue(HasAlertProperty); }
        protected set { SetValue(HasAlertPropertyKey, value); }
    }

    public IAlertable AlertContext
    {
        get { return (IAlertable )GetValue(AlertContextProperty); }
        set { SetValue(AlertContextProperty, value); }
    }

    private static void OnAlertContextChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
    {
        var obj = (AlertButton)d;

        obj.OnAlertContextChanged((IAlertable)e.OldValue, (IAlertable)e.NewValue);
    }

    private void OnAlertContextChanged(IAlertable prev, IAlertable curr)
    {
        if (prev != null)
        {
             UnhookEvents(prev);
        }
        if (curr != null)
        {
             HookEvents(curr);
        }
    }

    private void UnhookEvents(IAlertable context)
    {
        var handler = PropertyChangedEventHandler;
        if (handler != null)
        {
            context.PropertyChanged -= handler;
            PropertyChangedEventHandler = null;
        }

        UpdateDependantProperties();
    }

    private void HookEvents(IAlertable context)
    {
        var handler = new PropertyChangedEventHandler(OnPropertyChanged);
        PropertyChangedEventHandler = handler;

        context.PropertyChanged += handler;

        UpdateDependantProperties();
    }
}

---

The part I'm worried about is that my custom control subscribes/unsubscribes from the AlertContext's PropertyChanged event. But, what happens if I have the following scenario:

public sealed class MasterViewModel : ViewModelBase
{
    public ReadOnlyObservableCollection<DetailViewModel> { get; private set; }

    // etc...
}

public sealed class DetailViewModel : ViewModelBase
{
    public IAlertable AlertableObject { get; private set; }
    // etc...
}

Now, whenever I switch between detailed views, the user interface will create new AlertButtons each time, each of which will subscribe to the AlertableObject, but when I keep switching back and forth those objects are the same.

The reason I built things out the way I did in my custom control was because I saw that Microsoft did something similar with ButtonBase.Command:

    private static readonly UncommonField<EventHandler> CanExecuteChangedHandler = new UncommonField<EventHandler>();

    private void UnhookCommand(ICommand command)
    {
        EventHandler handler = CanExecuteChangedHandler.GetValue(this); 
        if (handler != null)
        { 
            command.CanExecuteChanged -= handler; 
            CanExecuteChangedHandler.ClearValue(this);
        } 
        UpdateCanExecute();
    }

    private void HookCommand(ICommand command) 
    {
        EventHandler handler = new EventHandler(OnCanExecuteChanged); 
        CanExecuteChangedHandler.SetValue(this, handler); 
        command.CanExecuteChanged += handler;
        UpdateCanExecute(); 
    }

The only part that is strange (I don't fully understand it) is UncommonField<T>, but it still stands that the ICommand.CanExecuteChanged event will still have a subscribed handler that is a property of the ButtonBase object. So, what am I missing here?

Answer 1

Yes, you are correct, in some cases ICommand can leak memory too! Usually it does not happen, because most implementations of ICommand either utilize CommandManager (which uses weak references) or do not care about CanExecute state and therefore use empty CanExecuteChanged event, which does not hold any references at all. But if you implement CanExecuteChanged as strong event, ICommand will cause memory leak.

In your case the simplest solution would probably be to additionally UnhookEvents when control is unloaded, and to HookEvents when it is loaded.

Alternatively, you can use weak events pattern as Zache suggested.