Browse Source

Use WeakReference in BindingNotification.

So that it doesn't keep objects alive when cached by
`.Publish().RefCount()` in `ExpressionObserver`. Added a leak test to
test that.
pull/691/head
Steven Kirk 10 years ago
parent
commit
a560c3b6d3
  1. 2
      src/Avalonia.Base/AvaloniaObject.cs
  2. 67
      src/Avalonia.Base/Data/BindingNotification.cs
  3. 2
      src/Avalonia.Base/PriorityValue.cs
  4. 19
      src/Markup/Avalonia.Markup/Data/ExpressionSubject.cs
  5. 18
      tests/Avalonia.LeakTests/ExpressionObserverTests.cs
  6. 5
      tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs

2
src/Avalonia.Base/AvaloniaObject.cs

@ -577,7 +577,7 @@ namespace Avalonia
{
if (notification.HasValue)
{
notification.Value = TypeUtilities.CastOrDefault(notification.Value, type);
notification.SetValue(TypeUtilities.CastOrDefault(notification.Value, type));
}
return notification;

67
src/Avalonia.Base/Data/BindingNotification.cs

@ -44,14 +44,19 @@ namespace Avalonia.Data
public static readonly BindingNotification UnsetValue =
new BindingNotification(AvaloniaProperty.UnsetValue);
// Null cannot be held in WeakReference as it's indistinguishable from an expired value so
// use this value in its place.
private static readonly object NullValue = new object();
private WeakReference<object> _value;
/// <summary>
/// Initializes a new instance of the <see cref="BindingNotification"/> class.
/// </summary>
/// <param name="value">The binding value.</param>
public BindingNotification(object value)
{
Value = value;
HasValue = true;
_value = new WeakReference<object>(value ?? NullValue);
}
/// <summary>
@ -66,7 +71,6 @@ namespace Avalonia.Data
throw new ArgumentException($"'errorType' may not be None");
}
Value = AvaloniaProperty.UnsetValue;
Error = error;
ErrorType = errorType;
}
@ -80,20 +84,42 @@ namespace Avalonia.Data
public BindingNotification(Exception error, BindingErrorType errorType, object fallbackValue)
: this(error, errorType)
{
Value = fallbackValue;
HasValue = true;
_value = new WeakReference<object>(fallbackValue ?? NullValue);
}
/// <summary>
/// Gets the value that should be passed to the target when <see cref="HasValue"/>
/// is true.
/// </summary>
public object Value { get; set; }
/// <remarks>
/// If this property is read when <see cref="HasValue"/> is false then it will return
/// <see cref="AvaloniaProperty.UnsetValue"/>.
/// </remarks>
public object Value
{
get
{
if (_value != null)
{
object result;
if (_value.TryGetTarget(out result))
{
return result == NullValue ? null : result;
}
}
// There's the possibility of a race condition in that HasValue can return true,
// and then the value is GC'd before Value is read. We should be ok though as
// we return UnsetValue which should be a safe alternative.
return AvaloniaProperty.UnsetValue;
}
}
/// <summary>
/// Gets a value indicating whether <see cref="Value"/> should be pushed to the target.
/// </summary>
public bool HasValue { get; set; }
public bool HasValue => _value != null;
/// <summary>
/// Gets the error that occurred on the source, if any.
@ -179,14 +205,7 @@ namespace Avalonia.Data
Contract.Requires<ArgumentNullException>(e != null);
Contract.Requires<ArgumentException>(type != BindingErrorType.None);
if (Error != null)
{
Error = new AggregateException(Error, e);
}
else
{
Error = e;
}
Error = Error != null ? new AggregateException(Error, e) : e;
if (type == BindingErrorType.Error || ErrorType == BindingErrorType.Error)
{
@ -194,10 +213,26 @@ namespace Avalonia.Data
}
}
/// <summary>
/// Removes the <see cref="Value"/> and makes <see cref="HasValue"/> return null.
/// </summary>
public void ClearValue()
{
_value = null;
}
/// <summary>
/// Sets the <see cref="Value"/>.
/// </summary>
public void SetValue(object value)
{
_value = new WeakReference<object>(value ?? NullValue);
}
private static bool ExceptionEquals(Exception a, Exception b)
{
return a?.GetType() == b?.GetType() &&
a.Message == b.Message;
a?.Message == b?.Message;
}
}
}

2
src/Avalonia.Base/PriorityValue.cs

@ -259,7 +259,7 @@ namespace Avalonia
if (notification?.HasValue == true)
{
notification.Value = castValue;
notification.SetValue(castValue);
}
if (notification == null || notification.HasValue)

19
src/Markup/Avalonia.Markup/Data/ExpressionSubject.cs

@ -255,7 +255,7 @@ namespace Avalonia.Markup.Data
}
}
private BindingNotification Merge(object a, BindingNotification b)
private static BindingNotification Merge(object a, BindingNotification b)
{
var an = a as BindingNotification;
@ -270,7 +270,7 @@ namespace Avalonia.Markup.Data
}
}
private BindingNotification Merge(BindingNotification a, object b)
private static BindingNotification Merge(BindingNotification a, object b)
{
var bn = b as BindingNotification;
@ -280,17 +280,22 @@ namespace Avalonia.Markup.Data
}
else
{
a.Value = b;
a.HasValue = true;
a.SetValue(b);
}
return a;
}
private BindingNotification Merge(BindingNotification a, BindingNotification b)
private static BindingNotification Merge(BindingNotification a, BindingNotification b)
{
a.Value = b.Value;
a.HasValue = b.HasValue;
if (b.HasValue)
{
a.SetValue(b.Value);
}
else
{
a.ClearValue();
}
if (b.Error != null)
{

18
tests/Avalonia.LeakTests/ExpressionObserverTests.cs

@ -35,6 +35,24 @@ namespace Avalonia.LeakTests
Assert.Equal(0, memory.GetObjects(where => where.Type.Is<AvaloniaList<string>>()).ObjectsCount));
}
[Fact]
public void Should_Not_Keep_Source_Alive_ObservableCollection_With_DataValidation()
{
Func<ExpressionObserver> run = () =>
{
var source = new { Foo = new AvaloniaList<string> { "foo", "bar" } };
var target = new ExpressionObserver(source, "Foo", true);
target.Subscribe(_ => { });
return target;
};
var result = run();
dotMemory.Check(memory =>
Assert.Equal(0, memory.GetObjects(where => where.Type.Is<AvaloniaList<string>>()).ObjectsCount));
}
[Fact]
public void Should_Not_Keep_Source_Alive_NonIntegerIndexer()
{

5
tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs

@ -166,15 +166,16 @@ namespace Avalonia.Markup.Xaml.UnitTests.Data
[Fact]
public void DataContext_Binding_Should_Produce_Correct_Results()
{
var viewModel = new { Foo = "bar" };
var root = new Decorator
{
DataContext = new { Foo = "bar" },
DataContext = viewModel,
};
var child = new Control();
var values = new List<object>();
child.GetObservable(Border.DataContextProperty).Subscribe(x => values.Add(x));
child.GetObservable(Control.DataContextProperty).Subscribe(x => values.Add(x));
child.Bind(Control.DataContextProperty, new Binding("Foo"));
// When binding to DataContext and the target isn't found, the binding should produce

Loading…
Cancel
Save