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 AlertButton
s 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?
-
2\$\begingroup\$ 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. \$\endgroup\$Zache– Zache2014年06月17日 11:51:42 +00:00Commented Jun 17, 2014 at 11:51
1 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.
Explore related questions
See similar questions with these tags.