Merge pull request #2984 from AvaloniaUI/fixes/2912-onewaytosource-stackoverflow

Fix some issues in OneWayToSource and OneTime bindings

samples/BindingDemo/MainWindow.xaml

@@ -24,6 +24,9 @@
             <TextBox Watermark="Two Way" UseFloatingWatermark="True" Text="{Binding Path=StringValue}" Name="first"/>
             <TextBox Watermark="One Way" UseFloatingWatermark="True" Text="{Binding Path=StringValue, Mode=OneWay}"/>
             <TextBox Watermark="One Time" UseFloatingWatermark="True" Text="{Binding Path=StringValue, Mode=OneTime}"/>
+            <!-- Removed due to #2983: reinstate when that's fixed.
+              <TextBox Watermark="One Way to Source" UseFloatingWatermark="True" Text="{Binding Path=StringValue, Mode=OneWayToSource}"/>
+            -->
           </StackPanel>
           <StackPanel Margin="18" Spacing="4" Width="200">
             <TextBlock FontSize="16" Text="Collection Bindings"/>

src/Avalonia.Base/Data/Core/ExpressionNode.cs

@@ -8,9 +8,13 @@ namespace Avalonia.Data.Core
     public abstract class ExpressionNode
     {
         private static readonly object CacheInvalid = new object();
+
         protected static readonly WeakReference<object> UnsetReference = 
             new WeakReference<object>(AvaloniaProperty.UnsetValue);
 
+        protected static readonly WeakReference<object> NullReference =
+            new WeakReference<object>(null);
+
         private WeakReference<object> _target = UnsetReference;
         private Action<object> _subscriber;
         private bool _listening;
@@ -98,7 +102,7 @@ namespace Avalonia.Data.Core
 
             if (notification == null)
             {
-                LastValue = new WeakReference<object>(value);
+                LastValue = value != null ? new WeakReference<object>(value) : NullReference;
 
                 if (Next != null)
                 {
@@ -111,7 +115,7 @@ namespace Avalonia.Data.Core
             }
             else
             {
-                LastValue = new WeakReference<object>(notification.Value);
+                LastValue = notification.Value != null ? new WeakReference<object>(notification.Value) : NullReference;
 
                 if (Next != null)
                 {
@@ -136,8 +140,8 @@ namespace Avalonia.Data.Core
             }
             else if (target != AvaloniaProperty.UnsetValue)
             {
-                StartListeningCore(_target);
                 _listening = true;
+                StartListeningCore(_target);
             }
             else
             {

src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs

@@ -103,8 +103,8 @@ namespace Avalonia.Data.Core.Plugins
 
             protected override void SubscribeCore()
             {
-                SendCurrentValue();
                 SubscribeToChanges();
+                SendCurrentValue();
             }
 
             protected override void UnsubscribeCore()

src/Avalonia.Base/Data/Core/SettableNode.cs

@@ -29,6 +29,11 @@ namespace Avalonia.Data.Core
 
             if (!isLastValueAlive)
             {
+                if (value == null && LastValue == NullReference)
+                {
+                    return true;
+                }
+
                 return false;
             }
 

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

@@ -154,6 +154,18 @@ namespace Avalonia.Markup.UnitTests.Data
             Assert.Equal("bar", source.Foo);
         }
 
+        [Fact]
+        public void OneTime_Binding_Releases_Subscription_If_DataContext_Set_Later()
+        {
+            var target = new TextBlock();
+            var source = new Source { Foo = "foo" };
+
+            target.Bind(TextBlock.TextProperty, new Binding("Foo", BindingMode.OneTime));
+            target.DataContext = source;
+
+            Assert.Equal(0, source.SubscriberCount);
+        }
+
         [Fact]
         public void OneWayToSource_Binding_Should_Be_Set_Up()
         {
@@ -196,6 +208,30 @@ namespace Avalonia.Markup.UnitTests.Data
             Assert.Equal("baz", target.Text);
         }
 
+        [Fact]
+        public void OneWayToSource_Binding_Should_Not_StackOverflow_With_Null_Value()
+        {
+            // Issue #2912
+            var target = new TextBlock { Text = null };
+            var binding = new Binding
+            {
+                Path = "Foo",
+                Mode = BindingMode.OneWayToSource,
+            };
+
+            target.Bind(TextBox.TextProperty, binding);
+
+            var source = new Source { Foo = "foo" };
+            target.DataContext = source;
+
+            Assert.Null(source.Foo);
+
+            // When running tests under NCrunch, NCrunch replaces the standard StackOverflowException
+            // with its own, which will be caught by our code. Detect the stackoverflow anyway, by
+            // making sure the target property was only set once.
+            Assert.Equal(2, source.FooSetCount);
+        }
+
         [Fact]
         public void Default_BindingMode_Should_Be_Used()
         {
@@ -543,6 +579,23 @@ namespace Avalonia.Markup.UnitTests.Data
             Assert.Equal(expected, child.DoubleValue);
         }
 
+        [Fact]
+        public void Combined_OneTime_And_OneWayToSource_Bindings_Should_Release_Subscriptions()
+        {
+            var target1 = new TextBlock();
+            var target2 = new TextBlock();
+            var root = new Panel { Children = { target1, target2 } };
+            var source = new Source { Foo = "foo" };
+
+            using (target1.Bind(TextBlock.TextProperty, new Binding("Foo", BindingMode.OneTime)))
+            using (target2.Bind(TextBlock.TextProperty, new Binding("Foo", BindingMode.OneWayToSource)))
+            {
+                root.DataContext = source;
+            }
+
+            Assert.Equal(0, source.SubscriberCount);
+        }
+
         private class StyledPropertyClass : AvaloniaObject
         {
             public static readonly StyledProperty<double> DoubleValueProperty =
@@ -622,6 +675,7 @@ namespace Avalonia.Markup.UnitTests.Data
 
         public class Source : INotifyPropertyChanged
         {
+            private PropertyChangedEventHandler _propertyChanged;
             private string _foo;
 
             public string Foo
@@ -630,15 +684,25 @@ namespace Avalonia.Markup.UnitTests.Data
                 set
                 {
                     _foo = value;
+                    ++FooSetCount;
                     RaisePropertyChanged();
                 }
             }
 
-            public event PropertyChangedEventHandler PropertyChanged;
+            public int FooSetCount { get; private set; }
+
+
+            public int SubscriberCount { get; private set; }
+
+            public event PropertyChangedEventHandler PropertyChanged
+            {
+                add { _propertyChanged += value; ++SubscriberCount; }
+                remove { _propertyChanged += value; --SubscriberCount; }
+            }
 
             private void RaisePropertyChanged([CallerMemberName] string prop = "")
             {
-                PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(prop));
+                _propertyChanged?.Invoke(this, new PropertyChangedEventArgs(prop));
             }
         }