Fix bug in DelayedSetter and change recursive delayed setting to prevent possible issues down the road.

src/Avalonia.Base/AvaloniaObject.cs

@@ -563,34 +563,35 @@ namespace Avalonia
             {
                 if (!object.Equals(field, value))
                 {
-                    var old = field;
+                    SetAndRaiseCore(property, ref field, value);
-                    field = value;
-                    
-                    using (directDelayedSetter.MarkNotifying(property))
-                    {
-                        RaisePropertyChanged(property, old, value, BindingPriority.LocalValue); 
-                    }
 
-                    if (directDelayedSetter.HasPendingSet(property))
+                    while (directDelayedSetter.HasPendingSet(property))
                     {
-                        SetAndRaise(property, ref field, (T)directDelayedSetter.GetFirstPendingSet(property));
+                        SetAndRaiseCore(property, ref field, (T)directDelayedSetter.GetFirstPendingSet(property));
                     }
                     return true;
                 }
-                else
-                {
-                    return false;
-                }
             }
-            else
+            else if(!object.Equals(field, value))
             {
                 directDelayedSetter.AddPendingSet(property, value);
-                return false;
+            }
+            return false;
+        }
+
+        private void SetAndRaiseCore<T>(AvaloniaProperty<T> property, ref T field, T value)
+        {
+            var old = field;
+            field = value;
+
+            using (directDelayedSetter.MarkNotifying(property))
+            {
+                RaisePropertyChanged(property, old, value, BindingPriority.LocalValue);
             }
         }
 
-        protected void SetAndRaise<T>(AvaloniaProperty<T> property, Action<T, Action<Action>> setterCallback, T value, Action<T> delayedSet)
+        protected void SetAndRaise<T>(AvaloniaProperty<T> property, Action<T, Action<Action>> setterCallback, T value)
-            => directDelayedSetter.SetAndNotify(property, (val, notify) => setterCallback((T)val, notify), value, val => delayedSet((T)val));
+            => directDelayedSetter.SetAndNotify(property, (val, notify) => setterCallback((T)val, notify), value);
 
         /// <summary>
         /// Tries to cast a value to a type, taking into account that the value may be a

src/Avalonia.Base/PriorityValue.cs

@@ -237,66 +237,73 @@ namespace Avalonia
         {
             if (!delayedSetter.IsNotifying(this))
             {
-                var notification = value as BindingNotification;
+                value = UpdateValueCore(value, priority);
-                object castValue;
 
-                if (notification != null)
+                while (delayedSetter.HasPendingSet(this))
                 {
-                    value = (notification.HasValue) ? notification.Value : null;
+                    var pendingSet = delayedSetter.GetFirstPendingSet(this);
+                    UpdateValueCore(pendingSet.value, pendingSet.priority);
                 }
+            }
+            else if(!object.Equals(value, _value))
+            {
+                delayedSetter.AddPendingSet(this, (value, priority));
+            }
+        }
 
-                if (!object.Equals(value, _value) && TypeUtilities.TryConvertImplicit(_valueType, value, out castValue))
+        private object UpdateValueCore(object value, int priority)
-                {
+        {
-                    var old = _value;
+            var notification = value as BindingNotification;
+            object castValue;
 
-                    if (_validate != null && castValue != AvaloniaProperty.UnsetValue)
+            if (notification != null)
-                    {
+            {
-                        castValue = _validate(castValue);
+                value = (notification.HasValue) ? notification.Value : null;
-                    }
+            }
 
-                    ValuePriority = priority;
+            if (TypeUtilities.TryConvertImplicit(_valueType, value, out castValue))
-                    _value = castValue;
+            {
+                var old = _value;
 
-                    if (notification?.HasValue == true)
+                if (_validate != null && castValue != AvaloniaProperty.UnsetValue)
-                    {
+                {
-                        notification.SetValue(castValue);
+                    castValue = _validate(castValue);
-                    }
+                }
 
-                    if (notification == null || notification.HasValue)
+                ValuePriority = priority;
-                    {
+                _value = castValue;
-                        using (delayedSetter.MarkNotifying(this))
-                        {
-                            Owner?.Changed(this, old, _value); 
-                        }
 
-                        if (delayedSetter.HasPendingSet(this))
+                if (notification?.HasValue == true)
-                        {
+                {
-                            var pendingSet = delayedSetter.GetFirstPendingSet(this);
+                    notification.SetValue(castValue);
-                            UpdateValue(pendingSet.value, pendingSet.priority);
+                }
-                        }
-                    }
 
-                    if (notification != null)
+                if (notification == null || notification.HasValue)
+                {
+                    using (delayedSetter.MarkNotifying(this))
                     {
-                        Owner?.BindingNotificationReceived(this, notification);
+                        Owner?.Changed(this, old, _value);
                     }
                 }
-                else
+
+                if (notification != null)
                 {
-                    Logger.Error(
+                    Owner?.BindingNotificationReceived(this, notification);
-                        LogArea.Binding,
+                }
-                        Owner,
-                        "Binding produced invalid value for {$Property} ({$PropertyType}): {$Value} ({$ValueType})",
-                        Property.Name,
-                        _valueType,
-                        value,
-                        value?.GetType());
-                } 
             }
             else
             {
-                delayedSetter.AddPendingSet(this, (value, priority));
+                Logger.Error(
+                    LogArea.Binding,
+                    Owner,
+                    "Binding produced invalid value for {$Property} ({$PropertyType}): {$Value} ({$ValueType})",
+                    Property.Name,
+                    _valueType,
+                    value,
+                    value?.GetType());
             }
+
+            return value;
         }
     }
 }

src/Avalonia.Base/Utilities/DelayedSetter.cs

@@ -58,10 +58,9 @@ namespace Avalonia.Utilities
             return setRecords[property].PendingValues.Dequeue();
         }
 
-        public void SetAndNotify(T property, Action<TValue, Action<Action>> setterCallback, TValue value, Action<TValue> delayedSet)
+        public void SetAndNotify(T property, Action<TValue, Action<Action>> setterCallback, TValue value)
         {
             Contract.Requires<ArgumentNullException>(setterCallback != null);
-            Contract.Requires<ArgumentNullException>(delayedSet != null);
             if (!IsNotifying(property))
             {
                 setterCallback(value, notification =>
@@ -71,9 +70,15 @@ namespace Avalonia.Utilities
                         notification();
                     }
                 });
-                if (HasPendingSet(property))
+                while (HasPendingSet(property))
                 {
-                    delayedSet(GetFirstPendingSet(property));
+                    setterCallback(GetFirstPendingSet(property), notification =>
+                    {
+                        using (MarkNotifying(property))
+                        {
+                            notification();
+                        }
+                    });
                 }
             }
             else

src/Avalonia.Controls/Primitives/SelectingItemsControl.cs

@@ -162,7 +162,7 @@ namespace Avalonia.Controls.Primitives
                             notifierWrapper(() => RaisePropertyChanged(SelectedIndexProperty, old, effective, BindingPriority.LocalValue));
                             SelectedItem = ElementAt(Items, effective);
                         }
-                    }, value, val => SelectedIndex = val);
+                    }, value);
                 }
                 else
                 {
@@ -215,7 +215,7 @@ namespace Avalonia.Controls.Primitives
                                 SelectedItems.Clear();
                             }
                         }
-                    }, value, val => SelectedItem = val);
+                    }, value);
                 }
                 else
                 {

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs

@@ -399,12 +399,6 @@ namespace Avalonia.Base.UnitTests
 
             var target = new Class1();
 
-            //note: if the initialization of the child binding is here target/child binding work fine!!!
-            //var child = new Class1()
-            //{
-            //    [~~Class1.DoubleValueProperty] = target[~~Class1.DoubleValueProperty]
-            //};
-
             target.Bind(Class1.DoubleValueProperty,
                 new Binding("Value") { Mode = BindingMode.TwoWay, Source = viewModel });
 
@@ -429,7 +423,7 @@ namespace Avalonia.Base.UnitTests
             //here in real life stack overflow exception is thrown issue #855 and #824
             target.DoubleValue = 51.001;
 
-            Assert.Equal(3, viewModel.SetterInvokedCount);
+            Assert.Equal(2, viewModel.SetterInvokedCount);
 
             double expected = 51;
 

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs

@@ -452,12 +452,6 @@ namespace Avalonia.Base.UnitTests
 
             var target = new Class1();
 
-            //note: if the initialization of the child binding is here there is no stackoverflow!!!
-            //var child = new Class1()
-            //{
-            //    [~~Class1.DoubleValueProperty] = target[~~Class1.DoubleValueProperty]
-            //};
-
             target.Bind(Class1.DoubleValueProperty, new Binding("Value")
                                                     {
                                                         Mode = BindingMode.TwoWay,
@@ -485,7 +479,7 @@ namespace Avalonia.Base.UnitTests
             //here in real life stack overflow exception is thrown issue #855 and #824
             target.DoubleValue = 51.001;
 
-            Assert.Equal(3, viewModel.SetterInvokedCount);
+            Assert.Equal(2, viewModel.SetterInvokedCount);
 
             double expected = 51;