Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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?

share|improve this question
1  
You are looking for the Weak Event Pattern, where subscribers to events only hold Weak References which allows the publisher of the event to be garbage collected. –  Zache Jun 17 at 11:51
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.