Prevent stack overflow in two-way bindings. (#17073)

* Add failing test for #16746

* Always read the value from the target object.

The value must be read from the target object instead of using the value from the event because the value may have changed again between the time the event was raised and now: if that occurs in a two-way binding then we end up with a stack overflow.

Author: grokys

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

@@ -507,8 +507,10 @@ private void OnTargetPropertyChanged(object? sender, AvaloniaPropertyChangedEven
         Debug.Assert(_mode is BindingMode.TwoWay or BindingMode.OneWayToSource);
         Debug.Assert(UpdateSourceTrigger is UpdateSourceTrigger.PropertyChanged);
 
-        if (e.Property == TargetProperty)
-            WriteValueToSource(e.NewValue);
+        // The value must be read from the target object instead of using the value from the event
+        // because the value may have changed again between the time the event was raised and now.
+        if (e.Property == TargetProperty && TryGetTarget(out var target))
+            WriteValueToSource(target.GetValue(TargetProperty));
     }
 
     private object? ConvertFallback(object? fallback, string fallbackName)

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

@@ -667,6 +667,31 @@ public void OneWayToSource_Binding_Does_Not_Override_TwoWay_Binding()
             Assert.Equal("TwoWay", source.Foo);
         }
 
+        [Fact]
+        public void Target_Undoing_Property_Change_During_TwoWay_Binding_Does_Not_Cause_StackOverflow()
+        {
+            var source = new TestStackOverflowViewModel { BoolValue = true };
+            var target = new TwoWayBindingTest();
+
+            source.ResetSetterInvokedCount();
+
+            // The AlwaysFalse property is set to false in the PropertyChanged callback. Ensure
+            // that binding it to an initial `true` value with a two-way binding does not cause a
+            // stack overflow.
+            target.Bind(
+                TwoWayBindingTest.AlwaysFalseProperty,
+                new Binding(nameof(TestStackOverflowViewModel.BoolValue))
+                {
+                    Mode = BindingMode.TwoWay,
+                });
+
+            target.DataContext = source;
+
+            Assert.Equal(1, source.SetterInvokedCount);
+            Assert.False(source.BoolValue);
+            Assert.False(target.AlwaysFalse);
+        }
+
         private class StyledPropertyClass : AvaloniaObject
         {
             public static readonly StyledProperty<double> DoubleValueProperty =
@@ -725,10 +750,24 @@ private class TestStackOverflowViewModel : INotifyPropertyChanged
 
             public const int MaxInvokedCount = 1000;
 
+            private bool _boolValue;
             private double _value;
 
             public event PropertyChangedEventHandler PropertyChanged;
 
+            public bool BoolValue
+            {
+                get => _boolValue;
+                set
+                {
+                    if (_boolValue != value)
+                    {
+                        _boolValue = value;
+                        SetterInvokedCount++;
+                        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(BoolValue)));
+                    }
+                } }
+
             public double Value
             {
                 get => _value;
@@ -754,20 +793,38 @@ public double Value
                     }
                 }
             }
+
+            public void ResetSetterInvokedCount() => SetterInvokedCount = 0;
         }
 
         private class TwoWayBindingTest : Control
         {
+            public static readonly StyledProperty<bool> AlwaysFalseProperty =
+                AvaloniaProperty.Register<StyledPropertyClass, bool>(nameof(AlwaysFalse));
             public static readonly StyledProperty<string> TwoWayProperty =
                 AvaloniaProperty.Register<TwoWayBindingTest, string>(
                     "TwoWay",
                     defaultBindingMode: BindingMode.TwoWay);
 
+            public bool AlwaysFalse
+            {
+                get => GetValue(AlwaysFalseProperty);
+                set => SetValue(AlwaysFalseProperty, value);
+            }
+
             public string TwoWay
             {
                 get => GetValue(TwoWayProperty);
                 set => SetValue(TwoWayProperty, value);
             }
+
+            protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change)
+            {
+                base.OnPropertyChanged(change);
+
+                if (change.Property == AlwaysFalseProperty)
+                    SetCurrentValue(AlwaysFalseProperty, false);
+            }
         }
 
         public class Source : INotifyPropertyChanged