Pārlūkot izejas kodu

Fix SelectedItem stack overflow bug. Changes of this pattern are basically required for any direct property that is not using SetAndRaise.

Jeremy Koritzinsky 8 gadi atpakaļ
vecāks
revīzija
8eb355e99d

+ 15 - 9
src/Avalonia.Base/AvaloniaObject.cs

@@ -51,6 +51,12 @@ namespace Avalonia
         /// </summary>
         /// </summary>
         private EventHandler<AvaloniaPropertyChangedEventArgs> _propertyChanged;
         private EventHandler<AvaloniaPropertyChangedEventArgs> _propertyChanged;
 
 
+        /// <summary>
+        /// Delayed setter helper for direct properties. Used to fix #855.
+        /// </summary>
+        protected readonly DelayedSetter<AvaloniaProperty, object> directDelayedSetter = new DelayedSetter<AvaloniaProperty, object>();
+
+
         /// <summary>
         /// <summary>
         /// Initializes a new instance of the <see cref="AvaloniaObject"/> class.
         /// Initializes a new instance of the <see cref="AvaloniaObject"/> class.
         /// </summary>
         /// </summary>
@@ -510,7 +516,6 @@ namespace Avalonia
         {
         {
             Contract.Requires<ArgumentNullException>(property != null);
             Contract.Requires<ArgumentNullException>(property != null);
             VerifyAccess();
             VerifyAccess();
-            delayedSetter.SetNotifying(property, true);
 
 
             AvaloniaPropertyChangedEventArgs e = new AvaloniaPropertyChangedEventArgs(
             AvaloniaPropertyChangedEventArgs e = new AvaloniaPropertyChangedEventArgs(
                 this,
                 this,
@@ -537,12 +542,9 @@ namespace Avalonia
             finally
             finally
             {
             {
                 property.Notifying?.Invoke(this, false);
                 property.Notifying?.Invoke(this, false);
-                delayedSetter.SetNotifying(property, false);
             }
             }
         }
         }
 
 
-        private DelayedSetter<AvaloniaProperty> delayedSetter = new DelayedSetter<AvaloniaProperty>();
-
         /// <summary>
         /// <summary>
         /// Sets the backing field for a direct avalonia property, raising the 
         /// Sets the backing field for a direct avalonia property, raising the 
         /// <see cref="PropertyChanged"/> event if the value has changed.
         /// <see cref="PropertyChanged"/> event if the value has changed.
@@ -557,17 +559,21 @@ namespace Avalonia
         protected bool SetAndRaise<T>(AvaloniaProperty<T> property, ref T field, T value)
         protected bool SetAndRaise<T>(AvaloniaProperty<T> property, ref T field, T value)
         {
         {
             VerifyAccess();
             VerifyAccess();
-            if (!delayedSetter.IsNotifying(property))
+            if (!directDelayedSetter.IsNotifying(property))
             {
             {
                 if (!object.Equals(field, value))
                 if (!object.Equals(field, value))
                 {
                 {
                     var old = field;
                     var old = field;
                     field = value;
                     field = value;
-                    RaisePropertyChanged(property, old, value, BindingPriority.LocalValue);
+                    
+                    using (directDelayedSetter.MarkNotifying(property))
+                    {
+                        RaisePropertyChanged(property, old, value, BindingPriority.LocalValue); 
+                    }
 
 
-                    if (delayedSetter.HasPendingSet(property))
+                    if (directDelayedSetter.HasPendingSet(property))
                     {
                     {
-                        SetAndRaise(property, ref field, (T)delayedSetter.GetFirstPendingSet(property));
+                        SetAndRaise(property, ref field, (T)directDelayedSetter.GetFirstPendingSet(property));
                     }
                     }
                     return true;
                     return true;
                 }
                 }
@@ -578,7 +584,7 @@ namespace Avalonia
             }
             }
             else
             else
             {
             {
-                delayedSetter.RecordPendingSet(property, value);
+                directDelayedSetter.AddPendingSet(property, value);
                 return false;
                 return false;
             }
             }
         }
         }

+ 14 - 9
src/Avalonia.Base/DelayedSetter.cs → src/Avalonia.Base/Utilities/DelayedSetter.cs

@@ -1,40 +1,45 @@
 using System;
 using System;
 using System.Collections.Generic;
 using System.Collections.Generic;
+using System.Reactive.Disposables;
 using System.Text;
 using System.Text;
 
 
-namespace Avalonia
+namespace Avalonia.Utilities
 {
 {
-    class DelayedSetter<T>
+    public class DelayedSetter<T, TValue>
     {
     {
         private class SettingStatus
         private class SettingStatus
         {
         {
             public bool Notifying { get; set; }
             public bool Notifying { get; set; }
 
 
-            private Queue<object> pendingValues;
+            private Queue<TValue> pendingValues;
             
             
-            public Queue<object> PendingValues
+            public Queue<TValue> PendingValues
             {
             {
                 get
                 get
                 {
                 {
-                    return pendingValues ?? (pendingValues = new Queue<object>());
+                    return pendingValues ?? (pendingValues = new Queue<TValue>());
                 }
                 }
             }
             }
         }
         }
 
 
         private readonly Dictionary<T, SettingStatus> setRecords = new Dictionary<T, SettingStatus>();
         private readonly Dictionary<T, SettingStatus> setRecords = new Dictionary<T, SettingStatus>();
 
 
-        public void SetNotifying(T property, bool notifying)
+        public IDisposable MarkNotifying(T property)
         {
         {
+            Contract.Requires<InvalidOperationException>(!IsNotifying(property));
+
             if (!setRecords.ContainsKey(property))
             if (!setRecords.ContainsKey(property))
             {
             {
                 setRecords[property] = new SettingStatus();
                 setRecords[property] = new SettingStatus();
             }
             }
-            setRecords[property].Notifying = notifying;
+            setRecords[property].Notifying = true;
+
+            return Disposable.Create(() => setRecords[property].Notifying = false);
         }
         }
 
 
         public bool IsNotifying(T property) => setRecords.TryGetValue(property, out var value) && value.Notifying;
         public bool IsNotifying(T property) => setRecords.TryGetValue(property, out var value) && value.Notifying;
 
 
-        public void RecordPendingSet(T property, object value)
+        public void AddPendingSet(T property, TValue value)
         {
         {
             if (!setRecords.ContainsKey(property))
             if (!setRecords.ContainsKey(property))
             {
             {
@@ -48,7 +53,7 @@ namespace Avalonia
             return setRecords.ContainsKey(property) && setRecords[property].PendingValues.Count != 0;
             return setRecords.ContainsKey(property) && setRecords[property].PendingValues.Count != 0;
         }
         }
 
 
-        public object GetFirstPendingSet(T property)
+        public TValue GetFirstPendingSet(T property)
         {
         {
             return setRecords[property].PendingValues.Dequeue();
             return setRecords[property].PendingValues.Dequeue();
         }
         }

+ 34 - 18
src/Avalonia.Controls/Primitives/SelectingItemsControl.cs

@@ -183,30 +183,46 @@ namespace Avalonia.Controls.Primitives
             {
             {
                 if (_updateCount == 0)
                 if (_updateCount == 0)
                 {
                 {
-                    var old = SelectedItem;
-                    var index = IndexOf(Items, value);
-                    var effective = index != -1 ? value : null;
-
-                    if (!object.Equals(effective, old))
+                    if (!directDelayedSetter.IsNotifying(SelectedItemProperty))
                     {
                     {
-                        _selectedItem = effective;
-                        RaisePropertyChanged(SelectedItemProperty, old, effective, BindingPriority.LocalValue);
-                        SelectedIndex = index;
+                        var old = SelectedItem;
+                        var index = IndexOf(Items, value);
+                        var effective = index != -1 ? value : null;
 
 
-                        if (effective != null)
+                        if (!object.Equals(effective, old))
                         {
                         {
-                            if (SelectedItems.Count != 1 || SelectedItems[0] != effective)
+                            _selectedItem = effective;
+                            using (directDelayedSetter.MarkNotifying(SelectedItemProperty))
+                            {
+                                RaisePropertyChanged(SelectedItemProperty, old, effective, BindingPriority.LocalValue);
+                            }
+
+                            SelectedIndex = index;
+
+                            if (effective != null)
+                            {
+                                if (SelectedItems.Count != 1 || SelectedItems[0] != effective)
+                                {
+                                    _syncingSelectedItems = true;
+                                    SelectedItems.Clear();
+                                    SelectedItems.Add(effective);
+                                    _syncingSelectedItems = false;
+                                }
+                            }
+                            else if (SelectedItems.Count > 0)
                             {
                             {
-                                _syncingSelectedItems = true;
                                 SelectedItems.Clear();
                                 SelectedItems.Clear();
-                                SelectedItems.Add(effective);
-                                _syncingSelectedItems = false;
                             }
                             }
-                        }
-                        else if (SelectedItems.Count > 0)
-                        {
-                            SelectedItems.Clear();
-                        }
+
+                            if (directDelayedSetter.HasPendingSet(SelectedItemProperty))
+                            {
+                                SelectedItem = directDelayedSetter.GetFirstPendingSet(SelectedItemProperty);
+                            }
+                        } 
+                    }
+                    else
+                    {
+                        directDelayedSetter.AddPendingSet(SelectedItemProperty, value);
                     }
                     }
                 }
                 }
                 else
                 else