diff options
author | Jonathan Peppers <jonathan.peppers@gmail.com> | 2016-08-16 13:19:21 -0500 |
---|---|---|
committer | Jason Smith <jason.smith@xamarin.com> | 2016-08-16 11:19:21 -0700 |
commit | f6febd4c81a80430331c7fcdcc63271aa4fa636c (patch) | |
tree | 01445df8cb4cfbc13e49356eb80fd8810a9df474 /Xamarin.Forms.Core | |
parent | 30c0dcb949186c21c60c4c9ddf8a581d40a43662 (diff) | |
download | xamarin-forms-f6febd4c81a80430331c7fcdcc63271aa4fa636c.tar.gz xamarin-forms-f6febd4c81a80430331c7fcdcc63271aa4fa636c.tar.bz2 xamarin-forms-f6febd4c81a80430331c7fcdcc63271aa4fa636c.zip |
Fix for BindingExpression memory leak (#279)
* Unit test proving a memory leak with Binding
What we were seeing in our app was that Binding objects stay around when
bound to long-lived ViewModels, even when the View is long gone
* BindingExpression - INotifyPropertyChanged should use WeakReference
I had to make a WeakPropertyChangedProxy class for this, I could not
think of a way to get around creating a new object for this
Diffstat (limited to 'Xamarin.Forms.Core')
-rw-r--r-- | Xamarin.Forms.Core/BindingExpression.cs | 78 |
1 files changed, 69 insertions, 9 deletions
diff --git a/Xamarin.Forms.Core/BindingExpression.cs b/Xamarin.Forms.Core/BindingExpression.cs index 505fc584..5ce8cd87 100644 --- a/Xamarin.Forms.Core/BindingExpression.cs +++ b/Xamarin.Forms.Core/BindingExpression.cs @@ -90,9 +90,7 @@ namespace Xamarin.Forms part.TryGetValue(sourceObject, out sourceObject); } - var inpc = sourceObject as INotifyPropertyChanged; - if (inpc != null) - inpc.PropertyChanged -= part.ChangeHandler; + part.Unsubscribe(); } } @@ -147,9 +145,7 @@ namespace Xamarin.Forms var inpc = current as INotifyPropertyChanged; if (inpc != null && !ReferenceEquals(current, previous)) { - // If we're reapplying, we don't want to double subscribe - inpc.PropertyChanged -= part.ChangeHandler; - inpc.PropertyChanged += part.ChangeHandler; + part.Subscribe(inpc); } } @@ -410,11 +406,57 @@ namespace Xamarin.Forms public object Source { get; private set; } } + class WeakPropertyChangedProxy + { + WeakReference _source, _listener; + + public WeakPropertyChangedProxy(INotifyPropertyChanged source, PropertyChangedEventHandler listener) + { + source.PropertyChanged += OnPropertyChanged; + _source = new WeakReference(source); + _listener = new WeakReference(listener); + } + + public void Unsubscribe() + { + if (_source != null) + { + var source = _source.Target as INotifyPropertyChanged; + if (source != null) + { + source.PropertyChanged -= OnPropertyChanged; + } + _source = null; + _listener = null; + } + } + + private void OnPropertyChanged(object sender, PropertyChangedEventArgs e) + { + if (_listener != null) + { + var handler = _listener.Target as PropertyChangedEventHandler; + if (handler != null) + { + handler(sender, e); + } + else + { + Unsubscribe(); + } + } + else + { + Unsubscribe(); + } + } + } + class BindingExpressionPart { readonly BindingExpression _expression; - - public readonly PropertyChangedEventHandler ChangeHandler; + readonly PropertyChangedEventHandler _changeHandler; + WeakPropertyChangedProxy _listener; public BindingExpressionPart(BindingExpression expression, string content, bool isIndexer = false) { @@ -423,7 +465,25 @@ namespace Xamarin.Forms Content = content; IsIndexer = isIndexer; - ChangeHandler = PropertyChanged; + _changeHandler = PropertyChanged; + } + + public void Subscribe(INotifyPropertyChanged handler) + { + // If we're reapplying, we don't want to double subscribe + Unsubscribe(); + + _listener = new WeakPropertyChangedProxy(handler, _changeHandler); + } + + public void Unsubscribe() + { + var listener = _listener; + if (listener != null) + { + listener.Unsubscribe(); + _listener = null; + } } public object[] Arguments { get; set; } |