From 7555a65c4f5ea1f2adead51cab84e376fbc7f611 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 18:42:29 +0200 Subject: [PATCH 01/24] Reformat to startup --- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 40 +++++++++---------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 80e6db8f9b3..886ae8bd68d 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; @@ -22,10 +22,8 @@ public CopyOnWriteList() : this(null) public CopyOnWriteList(object syncRoot) { - if(syncRoot == null) - { - syncRoot = new Object(); - } + syncRoot ??= new object(); + _syncRoot = syncRoot; } @@ -34,7 +32,7 @@ public CopyOnWriteList(object syncRoot) /// A non-null _readonlyWrapper is a "Copy on Write" flag. /// Methods that change the list (eg. Add() and Remove()) are /// responsible for: - /// 1) Checking _readonlyWrapper and copying the list before modifing it. + /// 1) Checking _readonlyWrapper and copying the list before modifying it. /// 2) Clearing _readonlyWrapper. /// public ArrayList List @@ -43,9 +41,9 @@ public ArrayList List { ArrayList tempList; - lock(_syncRoot) + lock (_syncRoot) { - if(null == _readonlyWrapper) + if (null == _readonlyWrapper) _readonlyWrapper = ArrayList.ReadOnly(_LiveList); tempList = _readonlyWrapper; } @@ -60,12 +58,12 @@ public ArrayList List /// public virtual bool Add(object obj) { - Debug.Assert(null!=obj, "CopyOnWriteList.Add() should not be passed null."); - lock(_syncRoot) + Debug.Assert(null != obj, "CopyOnWriteList.Add() should not be passed null."); + lock (_syncRoot) { int index = Find(obj); - if(index >= 0) + if (index >= 0) return false; return Internal_Add(obj); @@ -79,14 +77,14 @@ public virtual bool Add(object obj) /// public virtual bool Remove(object obj) { - Debug.Assert(null!=obj, "CopyOnWriteList.Remove() should not be passed null."); - lock(_syncRoot) + Debug.Assert(null != obj, "CopyOnWriteList.Remove() should not be passed null."); + lock (_syncRoot) { int index = Find(obj); // If the object is not on the list then // we are done. (return false) - if(index < 0) + if (index < 0) return false; return RemoveAt(index); @@ -99,7 +97,7 @@ public virtual bool Remove(object obj) /// protected object SyncRoot { - get{ return _syncRoot; } + get { return _syncRoot; } } /// @@ -110,7 +108,7 @@ protected object SyncRoot /// protected ArrayList LiveList { - get{ return _LiveList; } + get { return _LiveList; } } /// @@ -143,9 +141,9 @@ protected bool Internal_Insert(int index, object obj) private int Find(object obj) { // syncRoot Lock MUST be held by the caller. - for(int i = 0; i < _LiveList.Count; i++) + for (int i = 0; i < _LiveList.Count; i++) { - if(obj == _LiveList[i]) + if (obj == _LiveList[i]) { return i; } @@ -163,7 +161,7 @@ private int Find(object obj) protected bool RemoveAt(int index) { // syncRoot Lock MUST be held by the caller. - if(index <0 || index >= _LiveList.Count ) + if (index < 0 || index >= _LiveList.Count) return false; DoCopyOnWriteCheck(); @@ -177,14 +175,14 @@ protected void DoCopyOnWriteCheck() // If we have exposed (given out) a readonly reference to this // version of the list, then clone a new internal copy and cut // the old version free. - if(null != _readonlyWrapper) + if (null != _readonlyWrapper) { _LiveList = (ArrayList)_LiveList.Clone(); _readonlyWrapper = null; } } - private object _syncRoot; + private readonly object _syncRoot; private ArrayList _LiveList = new ArrayList(); private ArrayList _readonlyWrapper; } From 0c2f1ce5ad0e41a20f880ac70326c63c28d08644 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 18:54:59 +0200 Subject: [PATCH 02/24] Replace ArrayList with List / ReadOnlyCollection --- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 29 ++++++++++--------- .../MS/Internal/WeakReferenceEnumerator.cs | 5 ++-- .../Shared/MS/Internal/WeakReferenceList.cs | 8 ++--- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 886ae8bd68d..5d62a62e948 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; +using System.Collections.ObjectModel; namespace MS.Internal { @@ -14,7 +15,7 @@ namespace MS.Internal /// cache then the list is copied before it is modified and the readonly list is /// released from the cache. /// - internal class CopyOnWriteList + internal class CopyOnWriteList { public CopyOnWriteList() : this(null) { @@ -35,16 +36,16 @@ public CopyOnWriteList(object syncRoot) /// 1) Checking _readonlyWrapper and copying the list before modifying it. /// 2) Clearing _readonlyWrapper. /// - public ArrayList List + public ReadOnlyCollection List { get { - ArrayList tempList; + ReadOnlyCollection tempList; lock (_syncRoot) { if (null == _readonlyWrapper) - _readonlyWrapper = ArrayList.ReadOnly(_LiveList); + _readonlyWrapper = _LiveList.AsReadOnly(); tempList = _readonlyWrapper; } return tempList; @@ -56,7 +57,7 @@ public ArrayList List /// Returns true if successfully added. /// Returns false if object is already on the list. /// - public virtual bool Add(object obj) + public virtual bool Add(T obj) { Debug.Assert(null != obj, "CopyOnWriteList.Add() should not be passed null."); lock (_syncRoot) @@ -75,7 +76,7 @@ public virtual bool Add(object obj) /// Returns true if successfully removed. /// Returns false if object was not on the list. /// - public virtual bool Remove(object obj) + public virtual bool Remove(T obj) { Debug.Assert(null != obj, "CopyOnWriteList.Remove() should not be passed null."); lock (_syncRoot) @@ -106,7 +107,7 @@ protected object SyncRoot /// any copy on write protection. So the caller must really know what /// they are doing. /// - protected ArrayList LiveList + protected List LiveList { get { return _LiveList; } } @@ -116,7 +117,7 @@ protected ArrayList LiveList /// Without any error checks. /// For use by derived classes that implement there own error checks. /// - protected bool Internal_Add(object obj) + protected bool Internal_Add(T obj) { DoCopyOnWriteCheck(); _LiveList.Add(obj); @@ -128,7 +129,7 @@ protected bool Internal_Add(object obj) /// Without any error checks. /// For use by derived classes that implement there own error checks. /// - protected bool Internal_Insert(int index, object obj) + protected bool Internal_Insert(int index, T obj) { DoCopyOnWriteCheck(); _LiveList.Insert(index, obj); @@ -138,12 +139,12 @@ protected bool Internal_Insert(int index, object obj) /// /// Find an object on the List. /// - private int Find(object obj) + private int Find(T obj) { // syncRoot Lock MUST be held by the caller. for (int i = 0; i < _LiveList.Count; i++) { - if (obj == _LiveList[i]) + if (obj.Equals(_LiveList[i])) { return i; } @@ -177,13 +178,13 @@ protected void DoCopyOnWriteCheck() // the old version free. if (null != _readonlyWrapper) { - _LiveList = (ArrayList)_LiveList.Clone(); + _LiveList = new List(_LiveList); _readonlyWrapper = null; } } private readonly object _syncRoot; - private ArrayList _LiveList = new ArrayList(); - private ArrayList _readonlyWrapper; + private List _LiveList = new List(); + private ReadOnlyCollection _readonlyWrapper; } } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs index a08c90fd421..82cd0d15160 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; +using System.Collections.ObjectModel; namespace MS.Internal { @@ -18,7 +19,7 @@ namespace MS.Internal /// internal struct WeakReferenceListEnumerator : IEnumerator { - public WeakReferenceListEnumerator( ArrayList List) + public WeakReferenceListEnumerator( ReadOnlyCollection List) { _i = 0; _List = List; @@ -61,7 +62,7 @@ public void Reset() } private int _i; - private ArrayList _List; + private ReadOnlyCollection _List; private object _StrongReference; } } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index 15d712abd52..6ed19087345 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; @@ -14,7 +14,7 @@ namespace MS.Internal /// cache then the list is copied before it is modified and the readonly list is /// released from the cache. /// - internal class WeakReferenceList : CopyOnWriteList, IEnumerable + internal class WeakReferenceList : CopyOnWriteList, IEnumerable { public WeakReferenceList():base(null) { @@ -162,7 +162,7 @@ private int FindWeakReference(object obj) while (foundDeadReferences) { foundDeadReferences = false; - ArrayList list = base.LiveList; + List list = base.LiveList; for(int i = 0; i < list.Count; i++) { @@ -203,7 +203,7 @@ private int FindWeakReference(object obj) // caller is expected to lock the SyncRoot private void Purge() { - ArrayList list = base.LiveList; + List list = base.LiveList; int destIndex; int n = list.Count; From 1a991d22a1ab9bc93e0d3be2d37ce1b4dc069b64 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 19:06:01 +0200 Subject: [PATCH 03/24] Seal WeakReferenceList, clean up some checks --- .../Shared/MS/Internal/WeakReferenceList.cs | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index 6ed19087345..dec4b065f37 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -14,13 +14,13 @@ namespace MS.Internal /// cache then the list is copied before it is modified and the readonly list is /// released from the cache. /// - internal class WeakReferenceList : CopyOnWriteList, IEnumerable + internal sealed class WeakReferenceList : CopyOnWriteList, IEnumerable { - public WeakReferenceList():base(null) + public WeakReferenceList() : base(null) { } - public WeakReferenceList(object syncRoot):base(syncRoot) + public WeakReferenceList(object syncRoot) : base(syncRoot) { } @@ -36,7 +36,8 @@ IEnumerator IEnumerable.GetEnumerator() public bool Contains(object item) { - Debug.Assert(null != item, "WeakReferenceList.Contains() should not be passed null."); + Debug.Assert(item is not null, "WeakReferenceList.Contains() should not be passed null."); + lock (base.SyncRoot) { int index = FindWeakReference(item); @@ -70,22 +71,24 @@ public int Count /// public override bool Add(object obj) { - Debug.Assert(null!=obj, "WeakReferenceList.Add() should not be passed null."); + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); + return Add(obj, false /*skipFind*/); } - //Will insert a new WeakREference into the list. + //Will insert a new WeakReference into the list. //The object bein inserted MUST be unique as there is no check for it. public bool Add(object obj, bool skipFind) { - Debug.Assert(null!=obj, "WeakReferenceList.Add() should not be passed null."); - lock(base.SyncRoot) + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); + + lock (base.SyncRoot) { if (skipFind) { // before growing the list, purge it of dead entries. // The expense of purging amortizes to O(1) per entry, because - // the the list doubles its capacity when it grows. + // the list doubles its capacity when it grows. if (LiveList.Count == LiveList.Capacity) { Purge(); @@ -101,14 +104,15 @@ public bool Add(object obj, bool skipFind) } /// - /// Remove a weak reference to the List. - /// Returns true if successfully added. - /// Returns false if object is already on the list. + /// Remove a weak reference to the List. + /// Returns true if successfully removed. + /// Returns false if object is not in the list. /// public override bool Remove(object obj) { - Debug.Assert(null!=obj, "WeakReferenceList.Remove() should not be passed null."); - lock(base.SyncRoot) + Debug.Assert(obj is not null, "WeakReferenceList.Remove() should not be passed null."); + + lock (base.SyncRoot) { int index = FindWeakReference(obj); @@ -128,8 +132,9 @@ public override bool Remove(object obj) /// public bool Insert(int index, object obj) { - Debug.Assert(null!=obj, "WeakReferenceList.Add() should not be passed null."); - lock(base.SyncRoot) + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); + + lock (base.SyncRoot) { int existingIndex = FindWeakReference(obj); From d4d423038b8681aae04f78438e568502b220817f Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 19:06:54 +0200 Subject: [PATCH 04/24] Nullable context on WeakReferenceEnumerator, make properties readonly --- .../MS/Internal/WeakReferenceEnumerator.cs | 104 +++++++++--------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs index 82cd0d15160..36d697266e7 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs @@ -4,66 +4,66 @@ using System.Collections; using System.Collections.ObjectModel; -namespace MS.Internal +#nullable enable + +namespace MS.Internal; + +/// +/// This allows callers to "foreach" through a WeakReferenceList. +/// Each weakreference is checked for liveness and "current" +/// actually returns a strong reference to the current element. +/// +/// +/// Due to the way enumerators function, this enumerator often +/// holds a cached strong reference to the "Current" element. +/// This should not be a problem unless the caller stops enumerating +/// before the end of the list AND holds the enumerator alive forever. +/// +internal struct WeakReferenceListEnumerator : IEnumerator { - /// - /// This allows callers to "foreach" through a WeakReferenceList. - /// Each weakreference is checked for liveness and "current" - /// actually returns a strong reference to the current element. - /// - /// - /// Due to the way enumerators function, this enumerator often - /// holds a cached strong reference to the "Current" element. - /// This should not be a problem unless the caller stops enumerating - /// before the end of the list AND holds the enumerator alive forever. - /// - internal struct WeakReferenceListEnumerator : IEnumerator + private readonly ReadOnlyCollection _backingList; + + private object? _strongReference; + private int _index; + + public WeakReferenceListEnumerator(ReadOnlyCollection backingList) { - public WeakReferenceListEnumerator( ReadOnlyCollection List) - { - _i = 0; - _List = List; - _StrongReference = null; - } + _index = 0; + _backingList = backingList; + _strongReference = null; + } - object IEnumerator.Current - { get{ return Current; } } - - public object Current - { - get - { - if( null == _StrongReference ) - { - throw new System.InvalidOperationException(SR.Enumerator_VerifyContext); - } - return _StrongReference; - } - } + readonly object IEnumerator.Current + { + get => Current; + } - public bool MoveNext() - { - object obj=null; - while( _i < _List.Count ) - { - WeakReference weakRef = (WeakReference) _List[ _i++ ]; - obj = weakRef.Target; - if(null != obj) - break; - } - _StrongReference = obj; - return (null != obj); - } + public readonly object Current + { + get => _strongReference ?? throw new InvalidOperationException(SR.Enumerator_VerifyContext); + } + + public bool MoveNext() + { + object? element = null; - public void Reset() + while (_index < _backingList.Count) { - _i = 0; - _StrongReference = null; + WeakReference weakRef = (WeakReference)_backingList[_index++]; + element = weakRef.Target; + if (element is not null) + break; } - private int _i; - private ReadOnlyCollection _List; - private object _StrongReference; + _strongReference = element; + + return element is not null; + } + + public void Reset() + { + _index = 0; + _strongReference = null; } } From c4c3136563dd1044c8981562eadef4c0967a814b Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 19:08:29 +0200 Subject: [PATCH 05/24] Remove "base" references --- .../Shared/MS/Internal/WeakReferenceList.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index dec4b065f37..9acd7c06045 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -38,7 +38,7 @@ public bool Contains(object item) { Debug.Assert(item is not null, "WeakReferenceList.Contains() should not be passed null."); - lock (base.SyncRoot) + lock (SyncRoot) { int index = FindWeakReference(item); @@ -56,9 +56,9 @@ public int Count get { int count = 0; - lock (base.SyncRoot) + lock (SyncRoot) { - count = base.LiveList.Count; + count = LiveList.Count; } return count; } @@ -82,7 +82,7 @@ public bool Add(object obj, bool skipFind) { Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); - lock (base.SyncRoot) + lock (SyncRoot) { if (skipFind) { @@ -99,7 +99,7 @@ public bool Add(object obj, bool skipFind) return false; } - return base.Internal_Add(new WeakReference(obj)); + return Internal_Add(new WeakReference(obj)); } } @@ -112,7 +112,7 @@ public override bool Remove(object obj) { Debug.Assert(obj is not null, "WeakReferenceList.Remove() should not be passed null."); - lock (base.SyncRoot) + lock (SyncRoot) { int index = FindWeakReference(obj); @@ -121,7 +121,7 @@ public override bool Remove(object obj) if(index < 0) return false; - return base.RemoveAt(index); + return RemoveAt(index); } } @@ -134,7 +134,7 @@ public bool Insert(int index, object obj) { Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); - lock (base.SyncRoot) + lock (SyncRoot) { int existingIndex = FindWeakReference(obj); @@ -143,7 +143,7 @@ public bool Insert(int index, object obj) if(existingIndex >= 0) return false; - return base.Internal_Insert(index, new WeakReference(obj)); + return Internal_Insert(index, new WeakReference(obj)); } } @@ -167,7 +167,7 @@ private int FindWeakReference(object obj) while (foundDeadReferences) { foundDeadReferences = false; - List list = base.LiveList; + List list = LiveList; for(int i = 0; i < list.Count; i++) { @@ -208,7 +208,7 @@ private int FindWeakReference(object obj) // caller is expected to lock the SyncRoot private void Purge() { - List list = base.LiveList; + List list = LiveList; int destIndex; int n = list.Count; @@ -226,7 +226,7 @@ private void Purge() // but if there is, check for copy-on-write DoCopyOnWriteCheck(); - list = base.LiveList; + list = LiveList; // move remaining valid entries toward the beginning, into one // contiguous block From f0705647f951e63f48fe8b7705c764563b944016 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 19:09:47 +0200 Subject: [PATCH 06/24] Fix formatting on WeakReferenceList --- .../Shared/MS/Internal/WeakReferenceList.cs | 403 +++++++++--------- 1 file changed, 201 insertions(+), 202 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index 9acd7c06045..6dc138cb775 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -3,257 +3,256 @@ using System.Collections; -namespace MS.Internal +namespace MS.Internal; + +/// +/// This is a Cached ThreadSafe ArrayList of WeakReferences. +/// - When the "List" property is requested a readonly reference to the +/// list is returned and a reference to the readonly list is cached. +/// - If the "List" is requested again, the same cached reference is returned. +/// - When the list is modified, if a readonly reference is present in the +/// cache then the list is copied before it is modified and the readonly list is +/// released from the cache. +/// +internal sealed class WeakReferenceList : CopyOnWriteList, IEnumerable { - /// - /// This is a Cached ThreadSafe ArrayList of WeakReferences. - /// - When the "List" property is requested a readonly reference to the - /// list is returned and a reference to the readonly list is cached. - /// - If the "List" is requested again, the same cached reference is returned. - /// - When the list is modified, if a readonly reference is present in the - /// cache then the list is copied before it is modified and the readonly list is - /// released from the cache. - /// - internal sealed class WeakReferenceList : CopyOnWriteList, IEnumerable + public WeakReferenceList() : base(null) { - public WeakReferenceList() : base(null) - { - } + } - public WeakReferenceList(object syncRoot) : base(syncRoot) - { - } + public WeakReferenceList(object syncRoot) : base(syncRoot) + { + } - public WeakReferenceListEnumerator GetEnumerator() - { - return new WeakReferenceListEnumerator(List); - } + public WeakReferenceListEnumerator GetEnumerator() + { + return new WeakReferenceListEnumerator(List); + } - IEnumerator IEnumerable.GetEnumerator() + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + public bool Contains(object item) + { + Debug.Assert(item is not null, "WeakReferenceList.Contains() should not be passed null."); + + lock (SyncRoot) { - return GetEnumerator(); + int index = FindWeakReference(item); + + // If the object is already on the list then + // return true + if (index >= 0) + return true; + + return false; } + } - public bool Contains(object item) + public int Count + { + get { - Debug.Assert(item is not null, "WeakReferenceList.Contains() should not be passed null."); - + int count = 0; lock (SyncRoot) { - int index = FindWeakReference(item); - - // If the object is already on the list then - // return true - if (index >= 0) - return true; - - return false; + count = LiveList.Count; } + return count; } + } + + /// + /// Add a weak reference to the List. + /// Returns true if successfully added. + /// Returns false if object is already on the list. + /// + public override bool Add(object obj) + { + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); - public int Count + return Add(obj, false /*skipFind*/); + } + + //Will insert a new WeakReference into the list. + //The object bein inserted MUST be unique as there is no check for it. + public bool Add(object obj, bool skipFind) + { + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); + + lock (SyncRoot) { - get + if (skipFind) { - int count = 0; - lock (SyncRoot) + // before growing the list, purge it of dead entries. + // The expense of purging amortizes to O(1) per entry, because + // the list doubles its capacity when it grows. + if (LiveList.Count == LiveList.Capacity) { - count = LiveList.Count; + Purge(); } - return count; } -} + else if (FindWeakReference(obj) >= 0) + { + return false; + } + + return Internal_Add(new WeakReference(obj)); + } + } + + /// + /// Remove a weak reference to the List. + /// Returns true if successfully removed. + /// Returns false if object is not in the list. + /// + public override bool Remove(object obj) + { + Debug.Assert(obj is not null, "WeakReferenceList.Remove() should not be passed null."); - /// - /// Add a weak reference to the List. - /// Returns true if successfully added. - /// Returns false if object is already on the list. - /// - public override bool Add(object obj) + lock (SyncRoot) { - Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); + int index = FindWeakReference(obj); - return Add(obj, false /*skipFind*/); -} + // If the object is not on the list then + // we are done. (return false) + if (index < 0) + return false; - //Will insert a new WeakReference into the list. - //The object bein inserted MUST be unique as there is no check for it. - public bool Add(object obj, bool skipFind) + return RemoveAt(index); + } + } + + /// + /// Insert a weak reference into the List. + /// Returns true if successfully inserted. + /// Returns false if object is already on the list. + /// + public bool Insert(int index, object obj) + { + Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); + + lock (SyncRoot) { - Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); + int existingIndex = FindWeakReference(obj); - lock (SyncRoot) + // If the object is already on the list then + // we are done. (return false) + if (existingIndex >= 0) + return false; + + return Internal_Insert(index, new WeakReference(obj)); + } + } + + /// + /// Find an object on the List. + /// Also cleans up dead weak references. + /// + private int FindWeakReference(object obj) + { + // syncRoot Lock MUST be held by the caller. + // Search the LiveList looking for the object, also remove any + // dead references we find. + // + // We use the "LiveList" to avoid snapping a Clone every time we + // Change something. + // To do this correctly you need to understand how the base class + // virtualizes the Copy On Write. + bool foundDeadReferences = true; // so that the while loop runs the first time + int foundItem = -1; + + while (foundDeadReferences) + { + foundDeadReferences = false; + List list = LiveList; + + for (int i = 0; i < list.Count; i++) { - if (skipFind) + WeakReference weakRef = (WeakReference)list[i]; + if (weakRef.IsAlive) { - // before growing the list, purge it of dead entries. - // The expense of purging amortizes to O(1) per entry, because - // the list doubles its capacity when it grows. - if (LiveList.Count == LiveList.Capacity) + if (obj == weakRef.Target) { - Purge(); + foundItem = i; + break; } } - else if (FindWeakReference(obj) >= 0) + else { - return false; + foundDeadReferences = true; } - - return Internal_Add(new WeakReference(obj)); } - } - - /// - /// Remove a weak reference to the List. - /// Returns true if successfully removed. - /// Returns false if object is not in the list. - /// - public override bool Remove(object obj) - { - Debug.Assert(obj is not null, "WeakReferenceList.Remove() should not be passed null."); - lock (SyncRoot) + if (foundDeadReferences) { - int index = FindWeakReference(obj); - - // If the object is not on the list then - // we are done. (return false) - if(index < 0) - return false; - - return RemoveAt(index); + // if there were dead references, take this opportunity + // to clean up _all_ the dead references. After doing this, + // the foundItem index is no longer valid, so we just + // compute it again. + // Most of the time we expect no dead references, so the while + // loop runs once and the for loop walks the list once. + // Occasionally there will be dead references - the while loop + // runs twice and the for loop walks the list twice. Purge + // also walks the list once, for a total of three times. + Purge(); } } - /// - /// Insert a weak reference into the List. - /// Returns true if successfully inserted. - /// Returns false if object is already on the list. - /// - public bool Insert(int index, object obj) - { - Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); - - lock (SyncRoot) - { - int existingIndex = FindWeakReference(obj); + return foundItem; + } - // If the object is already on the list then - // we are done. (return false) - if(existingIndex >= 0) - return false; + // purge the list of dead references + // caller is expected to lock the SyncRoot + private void Purge() + { + List list = LiveList; + int destIndex; + int n = list.Count; - return Internal_Insert(index, new WeakReference(obj)); - } + // skip over valid entries at the beginning of the list + for (destIndex = 0; destIndex < n; ++destIndex) + { + WeakReference wr = (WeakReference)list[destIndex]; + if (!wr.IsAlive) + break; } - /// - /// Find an object on the List. - /// Also cleans up dead weakreferences. - /// - private int FindWeakReference(object obj) - { - // syncRoot Lock MUST be held by the caller. - // Search the LiveList looking for the object, also remove any - // dead references we find. - // - // We use the "LiveList" to avoid snapping a Clone everytime we - // Change something. - // To do this correctly you need to understand how the base class - // virtualizes the Copy On Write. - bool foundDeadReferences = true; // so that the while loop runs the first time - int foundItem = -1; - - while (foundDeadReferences) - { - foundDeadReferences = false; - List list = LiveList; - - for(int i = 0; i < list.Count; i++) - { - WeakReference weakRef = (WeakReference) list[i]; - if(weakRef.IsAlive) - { - if(obj == weakRef.Target) - { - foundItem = i; - break; - } - } - else - { - foundDeadReferences = true; - } - } - - if (foundDeadReferences) - { - // if there were dead references, take this opportunity - // to clean up _all_ the dead references. After doing this, - // the foundItem index is no longer valid, so we just - // compute it again. - // Most of the time we expect no dead references, so the while - // loop runs once and the for loop walks the list once. - // Occasionally there will be dead references - the while loop - // runs twice and the for loop walks the list twice. Purge - // also walks the list once, for a total of three times. - Purge(); - } - } - - return foundItem; - } - - // purge the list of dead references - // caller is expected to lock the SyncRoot - private void Purge() - { - List list = LiveList; - int destIndex; - int n = list.Count; - - // skip over valid entries at the beginning of the list - for (destIndex=0; destIndex= n) - return; + // there may be nothing to purge + if (destIndex >= n) + return; - // but if there is, check for copy-on-write - DoCopyOnWriteCheck(); - list = LiveList; + // but if there is, check for copy-on-write + DoCopyOnWriteCheck(); + list = LiveList; - // move remaining valid entries toward the beginning, into one - // contiguous block - for (int i=destIndex+1; i.TrimExcess(), because we're - // probably in the situation where additions to the list are common. - int newCapacity = destIndex << 1; - if (newCapacity < list.Capacity) - { - list.Capacity = newCapacity; - } + // shrink the list if it would be less than half full otherwise. + // This is more liberal than List.TrimExcess(), because we're + // probably in the situation where additions to the list are common. + int newCapacity = destIndex << 1; + if (newCapacity < list.Capacity) + { + list.Capacity = newCapacity; } - } + } } } From 00984e860f4e2e232c5342be99567b9418087f92 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 19:14:36 +0200 Subject: [PATCH 07/24] more syntax changes in CopyOnWriteList --- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 5d62a62e948..1065e5df296 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -17,6 +17,11 @@ namespace MS.Internal /// internal class CopyOnWriteList { + private ReadOnlyCollection _readonlyWrapper; + private List _listList; + + private readonly object _syncRoot; + public CopyOnWriteList() : this(null) { } @@ -26,6 +31,7 @@ public CopyOnWriteList(object syncRoot) syncRoot ??= new object(); _syncRoot = syncRoot; + _listList = new List(); } /// @@ -44,10 +50,11 @@ public ReadOnlyCollection List lock (_syncRoot) { - if (null == _readonlyWrapper) - _readonlyWrapper = _LiveList.AsReadOnly(); + _readonlyWrapper ??= _listList.AsReadOnly(); + tempList = _readonlyWrapper; } + return tempList; } } @@ -59,7 +66,8 @@ public ReadOnlyCollection List /// public virtual bool Add(T obj) { - Debug.Assert(null != obj, "CopyOnWriteList.Add() should not be passed null."); + Debug.Assert(obj is not null, "CopyOnWriteList.Add() should not be passed null."); + lock (_syncRoot) { int index = Find(obj); @@ -78,7 +86,7 @@ public virtual bool Add(T obj) /// public virtual bool Remove(T obj) { - Debug.Assert(null != obj, "CopyOnWriteList.Remove() should not be passed null."); + Debug.Assert(obj is not null, "CopyOnWriteList.Remove() should not be passed null."); lock (_syncRoot) { int index = Find(obj); @@ -109,7 +117,7 @@ protected object SyncRoot /// protected List LiveList { - get { return _LiveList; } + get { return _listList; } } /// @@ -120,7 +128,9 @@ protected List LiveList protected bool Internal_Add(T obj) { DoCopyOnWriteCheck(); - _LiveList.Add(obj); + + _listList.Add(obj); + return true; } @@ -132,7 +142,9 @@ protected bool Internal_Add(T obj) protected bool Internal_Insert(int index, T obj) { DoCopyOnWriteCheck(); - _LiveList.Insert(index, obj); + + _listList.Insert(index, obj); + return true; } @@ -142,9 +154,9 @@ protected bool Internal_Insert(int index, T obj) private int Find(T obj) { // syncRoot Lock MUST be held by the caller. - for (int i = 0; i < _LiveList.Count; i++) + for (int i = 0; i < _listList.Count; i++) { - if (obj.Equals(_LiveList[i])) + if (obj.Equals(_listList[i])) { return i; } @@ -162,11 +174,13 @@ private int Find(T obj) protected bool RemoveAt(int index) { // syncRoot Lock MUST be held by the caller. - if (index < 0 || index >= _LiveList.Count) + if (index < 0 || index >= _listList.Count) return false; DoCopyOnWriteCheck(); - _LiveList.RemoveAt(index); + + _listList.RemoveAt(index); + return true; } @@ -176,15 +190,11 @@ protected void DoCopyOnWriteCheck() // If we have exposed (given out) a readonly reference to this // version of the list, then clone a new internal copy and cut // the old version free. - if (null != _readonlyWrapper) + if (_readonlyWrapper is not null) { - _LiveList = new List(_LiveList); + _listList = new List(_listList); _readonlyWrapper = null; } } - - private readonly object _syncRoot; - private List _LiveList = new List(); - private ReadOnlyCollection _readonlyWrapper; } } From 2973bff3cfa1756da6592bfe879b97369260a106 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 19:14:57 +0200 Subject: [PATCH 08/24] File-scoped namespace in CopyOnWriteList --- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 302 +++++++++--------- 1 file changed, 150 insertions(+), 152 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 1065e5df296..b884af53600 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -1,200 +1,198 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections; using System.Collections.ObjectModel; -namespace MS.Internal +namespace MS.Internal; + +/// +/// This is a ThreadSafe ArrayList that uses Copy On Write to support consistency. +/// - When the "List" property is requested a readonly reference to the +/// list is returned and a reference to the readonly list is cached. +/// - If the "List" is requested again, the same cached reference is returned. +/// - When the list is modified, if a readonly reference is present in the +/// cache then the list is copied before it is modified and the readonly list is +/// released from the cache. +/// +internal class CopyOnWriteList { - /// - /// This is a ThreadSafe ArrayList that uses Copy On Write to support consistency. - /// - When the "List" property is requested a readonly reference to the - /// list is returned and a reference to the readonly list is cached. - /// - If the "List" is requested again, the same cached reference is returned. - /// - When the list is modified, if a readonly reference is present in the - /// cache then the list is copied before it is modified and the readonly list is - /// released from the cache. - /// - internal class CopyOnWriteList - { - private ReadOnlyCollection _readonlyWrapper; - private List _listList; + private ReadOnlyCollection _readonlyWrapper; + private List _listList; - private readonly object _syncRoot; + private readonly object _syncRoot; - public CopyOnWriteList() : this(null) - { - } + public CopyOnWriteList() : this(null) + { + } - public CopyOnWriteList(object syncRoot) - { - syncRoot ??= new object(); + public CopyOnWriteList(object syncRoot) + { + syncRoot ??= new object(); - _syncRoot = syncRoot; - _listList = new List(); - } + _syncRoot = syncRoot; + _listList = new List(); + } - /// - /// Return a readonly wrapper of the list. Note: this is NOT a copy. - /// A non-null _readonlyWrapper is a "Copy on Write" flag. - /// Methods that change the list (eg. Add() and Remove()) are - /// responsible for: - /// 1) Checking _readonlyWrapper and copying the list before modifying it. - /// 2) Clearing _readonlyWrapper. - /// - public ReadOnlyCollection List + /// + /// Return a readonly wrapper of the list. Note: this is NOT a copy. + /// A non-null _readonlyWrapper is a "Copy on Write" flag. + /// Methods that change the list (eg. Add() and Remove()) are + /// responsible for: + /// 1) Checking _readonlyWrapper and copying the list before modifying it. + /// 2) Clearing _readonlyWrapper. + /// + public ReadOnlyCollection List + { + get { - get - { - ReadOnlyCollection tempList; + ReadOnlyCollection tempList; - lock (_syncRoot) - { - _readonlyWrapper ??= _listList.AsReadOnly(); - - tempList = _readonlyWrapper; - } + lock (_syncRoot) + { + _readonlyWrapper ??= _listList.AsReadOnly(); - return tempList; + tempList = _readonlyWrapper; } + + return tempList; } + } - /// - /// Add an object to the List. - /// Returns true if successfully added. - /// Returns false if object is already on the list. - /// - public virtual bool Add(T obj) - { - Debug.Assert(obj is not null, "CopyOnWriteList.Add() should not be passed null."); + /// + /// Add an object to the List. + /// Returns true if successfully added. + /// Returns false if object is already on the list. + /// + public virtual bool Add(T obj) + { + Debug.Assert(obj is not null, "CopyOnWriteList.Add() should not be passed null."); - lock (_syncRoot) - { - int index = Find(obj); + lock (_syncRoot) + { + int index = Find(obj); - if (index >= 0) - return false; + if (index >= 0) + return false; - return Internal_Add(obj); - } + return Internal_Add(obj); } + } - /// - /// Remove an object from the List. - /// Returns true if successfully removed. - /// Returns false if object was not on the list. - /// - public virtual bool Remove(T obj) + /// + /// Remove an object from the List. + /// Returns true if successfully removed. + /// Returns false if object was not on the list. + /// + public virtual bool Remove(T obj) + { + Debug.Assert(obj is not null, "CopyOnWriteList.Remove() should not be passed null."); + lock (_syncRoot) { - Debug.Assert(obj is not null, "CopyOnWriteList.Remove() should not be passed null."); - lock (_syncRoot) - { - int index = Find(obj); + int index = Find(obj); - // If the object is not on the list then - // we are done. (return false) - if (index < 0) - return false; + // If the object is not on the list then + // we are done. (return false) + if (index < 0) + return false; - return RemoveAt(index); - } + return RemoveAt(index); } + } - /// - /// This allows derived classes to take the lock. This is mostly used - /// to extend Add() and Remove() etc. - /// - protected object SyncRoot - { - get { return _syncRoot; } - } + /// + /// This allows derived classes to take the lock. This is mostly used + /// to extend Add() and Remove() etc. + /// + protected object SyncRoot + { + get { return _syncRoot; } + } - /// - /// This is protected and the caller can get into real serious trouble - /// using this. Because this points at the real current list without - /// any copy on write protection. So the caller must really know what - /// they are doing. - /// - protected List LiveList - { - get { return _listList; } - } + /// + /// This is protected and the caller can get into real serious trouble + /// using this. Because this points at the real current list without + /// any copy on write protection. So the caller must really know what + /// they are doing. + /// + protected List LiveList + { + get { return _listList; } + } - /// - /// Add an object to the List. - /// Without any error checks. - /// For use by derived classes that implement there own error checks. - /// - protected bool Internal_Add(T obj) - { - DoCopyOnWriteCheck(); + /// + /// Add an object to the List. + /// Without any error checks. + /// For use by derived classes that implement there own error checks. + /// + protected bool Internal_Add(T obj) + { + DoCopyOnWriteCheck(); - _listList.Add(obj); + _listList.Add(obj); - return true; - } + return true; + } - /// - /// Insert an object into the List at the given index. - /// Without any error checks. - /// For use by derived classes that implement there own error checks. - /// - protected bool Internal_Insert(int index, T obj) - { - DoCopyOnWriteCheck(); + /// + /// Insert an object into the List at the given index. + /// Without any error checks. + /// For use by derived classes that implement there own error checks. + /// + protected bool Internal_Insert(int index, T obj) + { + DoCopyOnWriteCheck(); - _listList.Insert(index, obj); + _listList.Insert(index, obj); - return true; - } + return true; + } - /// - /// Find an object on the List. - /// - private int Find(T obj) + /// + /// Find an object on the List. + /// + private int Find(T obj) + { + // syncRoot Lock MUST be held by the caller. + for (int i = 0; i < _listList.Count; i++) { - // syncRoot Lock MUST be held by the caller. - for (int i = 0; i < _listList.Count; i++) + if (obj.Equals(_listList[i])) { - if (obj.Equals(_listList[i])) - { - return i; - } + return i; } - return -1; } + return -1; + } - /// - /// Remove the object at a given index from the List. - /// Returns true if successfully removed. - /// Returns false if index is outside the range of the list. - /// - /// This is protected because it operates on the LiveList - /// - protected bool RemoveAt(int index) - { - // syncRoot Lock MUST be held by the caller. - if (index < 0 || index >= _listList.Count) - return false; + /// + /// Remove the object at a given index from the List. + /// Returns true if successfully removed. + /// Returns false if index is outside the range of the list. + /// + /// This is protected because it operates on the LiveList + /// + protected bool RemoveAt(int index) + { + // syncRoot Lock MUST be held by the caller. + if (index < 0 || index >= _listList.Count) + return false; - DoCopyOnWriteCheck(); + DoCopyOnWriteCheck(); - _listList.RemoveAt(index); + _listList.RemoveAt(index); - return true; - } + return true; + } - protected void DoCopyOnWriteCheck() + protected void DoCopyOnWriteCheck() + { + // syncRoot Lock MUST be held by the caller. + // If we have exposed (given out) a readonly reference to this + // version of the list, then clone a new internal copy and cut + // the old version free. + if (_readonlyWrapper is not null) { - // syncRoot Lock MUST be held by the caller. - // If we have exposed (given out) a readonly reference to this - // version of the list, then clone a new internal copy and cut - // the old version free. - if (_readonlyWrapper is not null) - { - _listList = new List(_listList); - _readonlyWrapper = null; - } + _listList = new List(_listList); + _readonlyWrapper = null; } } } From 9c98e8f8786e8fa25202f0c7a3edbc95a5ce2df1 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 20:02:47 +0200 Subject: [PATCH 09/24] Convert WeakReferenceList/Enumerator as generic --- .../MS/Internal/WeakReferenceEnumerator.cs | 28 +++++++---- .../Shared/MS/Internal/WeakReferenceList.cs | 47 ++++++++++--------- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs index 36d697266e7..4358826aab4 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs @@ -19,39 +19,44 @@ namespace MS.Internal; /// This should not be a problem unless the caller stops enumerating /// before the end of the list AND holds the enumerator alive forever. /// -internal struct WeakReferenceListEnumerator : IEnumerator +internal struct WeakReferenceListEnumerator : IEnumerator where T : class { - private readonly ReadOnlyCollection _backingList; + private readonly ReadOnlyCollection> _backingList; - private object? _strongReference; + private T? _strongReference; private int _index; - public WeakReferenceListEnumerator(ReadOnlyCollection backingList) + internal WeakReferenceListEnumerator(ReadOnlyCollection> backingList) { _index = 0; _backingList = backingList; _strongReference = null; } + readonly T IEnumerator.Current + { + get => Current; + } + readonly object IEnumerator.Current { get => Current; } - public readonly object Current + public readonly T Current { get => _strongReference ?? throw new InvalidOperationException(SR.Enumerator_VerifyContext); } public bool MoveNext() { - object? element = null; + T? element = null; while (_index < _backingList.Count) { - WeakReference weakRef = (WeakReference)_backingList[_index++]; - element = weakRef.Target; - if (element is not null) + WeakReference weakRef = _backingList[_index++]; + + if (weakRef.TryGetTarget(out element)) break; } @@ -65,5 +70,10 @@ public void Reset() _index = 0; _strongReference = null; } + + public readonly void Dispose() + { + // This method is here to satisfy the IEnumerator interface. + } } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index 6dc138cb775..4d9757d7517 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -14,7 +14,7 @@ namespace MS.Internal; /// cache then the list is copied before it is modified and the readonly list is /// released from the cache. /// -internal sealed class WeakReferenceList : CopyOnWriteList, IEnumerable +internal sealed class WeakReferenceList : CopyOnWriteList>, IEnumerable where T : class { public WeakReferenceList() : base(null) { @@ -24,9 +24,9 @@ public WeakReferenceList(object syncRoot) : base(syncRoot) { } - public WeakReferenceListEnumerator GetEnumerator() + public WeakReferenceListEnumerator GetEnumerator() { - return new WeakReferenceListEnumerator(List); + return new WeakReferenceListEnumerator(List); } IEnumerator IEnumerable.GetEnumerator() @@ -34,7 +34,12 @@ IEnumerator IEnumerable.GetEnumerator() return GetEnumerator(); } - public bool Contains(object item) + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + public bool Contains(T item) { Debug.Assert(item is not null, "WeakReferenceList.Contains() should not be passed null."); @@ -69,16 +74,16 @@ public int Count /// Returns true if successfully added. /// Returns false if object is already on the list. /// - public override bool Add(object obj) + public bool Add(T obj) { Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); - return Add(obj, false /*skipFind*/); + return Add(obj, skipFind: false); } //Will insert a new WeakReference into the list. //The object bein inserted MUST be unique as there is no check for it. - public bool Add(object obj, bool skipFind) + public bool Add(T obj, bool skipFind) { Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); @@ -99,7 +104,7 @@ public bool Add(object obj, bool skipFind) return false; } - return Internal_Add(new WeakReference(obj)); + return Internal_Add(new WeakReference(obj)); } } @@ -108,7 +113,7 @@ public bool Add(object obj, bool skipFind) /// Returns true if successfully removed. /// Returns false if object is not in the list. /// - public override bool Remove(object obj) + public bool Remove(T obj) { Debug.Assert(obj is not null, "WeakReferenceList.Remove() should not be passed null."); @@ -130,7 +135,7 @@ public override bool Remove(object obj) /// Returns true if successfully inserted. /// Returns false if object is already on the list. /// - public bool Insert(int index, object obj) + public bool Insert(int index, T obj) { Debug.Assert(obj is not null, "WeakReferenceList.Add() should not be passed null."); @@ -143,7 +148,7 @@ public bool Insert(int index, object obj) if (existingIndex >= 0) return false; - return Internal_Insert(index, new WeakReference(obj)); + return Internal_Insert(index, new WeakReference(obj)); } } @@ -151,7 +156,7 @@ public bool Insert(int index, object obj) /// Find an object on the List. /// Also cleans up dead weak references. /// - private int FindWeakReference(object obj) + private int FindWeakReference(T obj) { // syncRoot Lock MUST be held by the caller. // Search the LiveList looking for the object, also remove any @@ -167,14 +172,14 @@ private int FindWeakReference(object obj) while (foundDeadReferences) { foundDeadReferences = false; - List list = LiveList; + List> list = LiveList; for (int i = 0; i < list.Count; i++) { - WeakReference weakRef = (WeakReference)list[i]; - if (weakRef.IsAlive) + WeakReference weakRef = list[i]; + if (weakRef.TryGetTarget(out T target)) { - if (obj == weakRef.Target) + if (obj == target) { foundItem = i; break; @@ -208,15 +213,15 @@ private int FindWeakReference(object obj) // caller is expected to lock the SyncRoot private void Purge() { - List list = LiveList; + List> list = LiveList; int destIndex; int n = list.Count; // skip over valid entries at the beginning of the list for (destIndex = 0; destIndex < n; ++destIndex) { - WeakReference wr = (WeakReference)list[destIndex]; - if (!wr.IsAlive) + WeakReference wr = list[destIndex]; + if (!wr.TryGetTarget(out _)) break; } @@ -232,8 +237,8 @@ private void Purge() // contiguous block for (int i = destIndex + 1; i < n; ++i) { - WeakReference wr = (WeakReference)list[i]; - if (wr.IsAlive) + WeakReference wr = list[i]; + if (wr.TryGetTarget(out _)) { list[destIndex++] = list[i]; } From e3286ae360fa22dbf4ae0c2b0fd25798a467f156 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 20:14:38 +0200 Subject: [PATCH 10/24] Use the perks of generic collections, avoid null-checks that are always passing --- .../System/Windows/PresentationSource.cs | 21 +++--- .../ResourceDictionaryDiagnostics.cs | 38 +++-------- .../System/Windows/ResourceDictionary.cs | 64 +++++++++---------- .../System/Windows/SystemResources.cs | 10 ++- .../src/Shared/MS/Win32/HwndWrapper.cs | 2 +- 5 files changed, 53 insertions(+), 82 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/PresentationSource.cs b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/PresentationSource.cs index ddd8abb434c..2c75428b282 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/PresentationSource.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/PresentationSource.cs @@ -449,7 +449,7 @@ protected void RootChanged(Visual oldRoot, Visual newRoot) // To fire PresentationSourceChanged when the RootVisual changes; // rather than simulate a "parent" pointer change, we just walk the // collection of all nodes that need the event. - foreach (DependencyObject element in _watchers) + foreach (DependencyObject element in s_watchers) { // We only need to update those elements that are in the // same context as this presentation source. @@ -475,7 +475,7 @@ protected void RootChanged(Visual oldRoot, Visual newRoot) /// protected void AddSource() { - _sources.Add(this); + s_sources.Add(this); } /// @@ -483,7 +483,7 @@ protected void AddSource() /// protected void RemoveSource() { - _sources.Remove(this); + s_sources.Remove(this); } /// @@ -613,17 +613,14 @@ internal static bool IsUnderSamePresentationSource(params ReadOnlySpan - internal static WeakReferenceList CriticalCurrentSources + internal static WeakReferenceList CriticalCurrentSources { - get - { - return _sources; - } + get => s_sources; } private static void AddElementToWatchList(DependencyObject element) { - if(_watchers.Add(element)) + if (s_watchers.Add(element)) { element.SetValue(CachedSourceProperty, PresentationSource.FindSource(element)); element.SetValue(GetsSourceChangedEventProperty, true); @@ -633,7 +630,7 @@ private static void AddElementToWatchList(DependencyObject element) private static void RemoveElementFromWatchList(DependencyObject element) { - if(_watchers.Remove(element)) + if (s_watchers.Remove(element)) { element.ClearValue(CachedSourceProperty); element.ClearValue(GetsSourceChangedEventProperty); @@ -741,11 +738,11 @@ private static readonly DependencyProperty CachedSourceProperty private static readonly object _globalLock = new object(); // An array of weak-references to sources that we know about. - private static WeakReferenceList _sources = new WeakReferenceList(_globalLock); + private static readonly WeakReferenceList s_sources = new(_globalLock); // An array of weak-references to elements that need to know // about source changes. - private static WeakReferenceList _watchers = new WeakReferenceList(_globalLock); + private static readonly WeakReferenceList s_watchers = new(_globalLock); #endregion } diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs index 6ea9c31af7d..6a5dcecc7bb 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // @@ -51,7 +51,7 @@ public static IEnumerable ThemedResourceDictionaries { if (!IsEnabled) { - return ResourceDictionaryDiagnostics.EmptyResourceDictionaryInfos; + return ReadOnlyCollection.Empty; } return SystemResources.ThemedResourceDictionaries; @@ -69,7 +69,7 @@ public static IEnumerable GenericResourceDictionaries { if (!IsEnabled) { - return ResourceDictionaryDiagnostics.EmptyResourceDictionaryInfos; + return ReadOnlyCollection.Empty; } return SystemResources.GenericResourceDictionaries; @@ -276,47 +276,31 @@ private static IReadOnlyCollection EmptyResourceDictionaries public static IEnumerable GetFrameworkElementOwners(ResourceDictionary dictionary) { - return GetOwners(dictionary.FrameworkElementOwners, EmptyFrameworkElementList); + return GetOwners(dictionary.FrameworkElementOwners); } public static IEnumerable GetFrameworkContentElementOwners(ResourceDictionary dictionary) { - return GetOwners(dictionary.FrameworkContentElementOwners, EmptyFrameworkContentElementList); + return GetOwners(dictionary.FrameworkContentElementOwners); } public static IEnumerable GetApplicationOwners(ResourceDictionary dictionary) { - return GetOwners(dictionary.ApplicationOwners, EmptyApplicationList); + return GetOwners(dictionary.ApplicationOwners); } - private static IEnumerable GetOwners(WeakReferenceList list, IEnumerable emptyList) + private static IEnumerable GetOwners(WeakReferenceList list) where T : DispatcherObject { if (!IsEnabled || list == null || list.Count == 0) { - return emptyList; + return Array.Empty(); } - List result = new List(list.Count); - foreach (Object o in list) - { - T owner = o as T; - if (owner != null) - { - result.Add(owner); - } - } - - return result.AsReadOnly(); + // Create a read-only copy of the list + return new List(list).AsReadOnly(); } - private static IReadOnlyCollection EmptyFrameworkElementList - => Array.Empty(); - private static IReadOnlyCollection EmptyFrameworkContentElementList - => Array.Empty(); - private static IReadOnlyCollection EmptyApplicationList - => Array.Empty(); - #endregion #region Notify when static resource references are resolved @@ -548,7 +532,5 @@ internal class LookupResult internal static bool IsEnabled { get; private set; } - private static readonly ReadOnlyCollection EmptyResourceDictionaryInfos - = new List().AsReadOnly(); } } diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs index 84e2c746bdf..510709d0550 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs @@ -1495,7 +1495,7 @@ internal void AddOwner(DispatcherObject owner) { if (_ownerFEs == null) { - _ownerFEs = new WeakReferenceList(1); + _ownerFEs = new WeakReferenceList(1); } else if (_ownerFEs.Contains(fe) && ContainsCycle(this)) { @@ -1517,7 +1517,7 @@ internal void AddOwner(DispatcherObject owner) { if (_ownerFCEs == null) { - _ownerFCEs = new WeakReferenceList(1); + _ownerFCEs = new WeakReferenceList(1); } else if (_ownerFCEs.Contains(fce) && ContainsCycle(this)) { @@ -1539,7 +1539,7 @@ internal void AddOwner(DispatcherObject owner) { if (_ownerApps == null) { - _ownerApps = new WeakReferenceList(1); + _ownerApps = new WeakReferenceList(1); } else if (_ownerApps.Contains(app) && ContainsCycle(this)) { @@ -1805,14 +1805,14 @@ private object FetchResource( return GetValue(resourceKey, out canCache); } - private WeakReferenceList GetOrCreateWeakReferenceList(object resourceKey) + private WeakReferenceList GetOrCreateWeakReferenceList(object resourceKey) { - this._weakDeferredResourceReferencesMap ??= new(); + _weakDeferredResourceReferencesMap ??= new Dictionary>(); - if (!this._weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences)) + if (!_weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences)) { - weakDeferredResourceReferences = new WeakReferenceList(); - this._weakDeferredResourceReferencesMap[resourceKey] = weakDeferredResourceReferences; + weakDeferredResourceReferences = new WeakReferenceList(); + _weakDeferredResourceReferencesMap[resourceKey] = weakDeferredResourceReferences; } return weakDeferredResourceReferences; @@ -1820,8 +1820,7 @@ private WeakReferenceList GetOrCreateWeakReferenceList(object resourceKey) internal void RemoveDeferredResourceReference(DeferredResourceReference deferredResourceReference) { - - if (this._weakDeferredResourceReferencesMap?.TryGetValue(deferredResourceReference.Key, out var weakDeferredResourceReferences) is true) + if (_weakDeferredResourceReferencesMap?.TryGetValue(deferredResourceReference.Key, out var weakDeferredResourceReferences) is true) { weakDeferredResourceReferences.Remove(deferredResourceReference); } @@ -2012,14 +2011,12 @@ private void PropagateParentOwners(ResourceDictionary mergedDictionary) if (mergedDictionary._ownerFEs == null) { - mergedDictionary._ownerFEs = new WeakReferenceList(_ownerFEs.Count); + mergedDictionary._ownerFEs = new WeakReferenceList(_ownerFEs.Count); } - foreach (object o in _ownerFEs) + foreach (FrameworkElement fe in _ownerFEs) { - FrameworkElement fe = o as FrameworkElement; - if (fe != null) - mergedDictionary.AddOwner(fe); + mergedDictionary.AddOwner(fe); } } @@ -2029,14 +2026,12 @@ private void PropagateParentOwners(ResourceDictionary mergedDictionary) if (mergedDictionary._ownerFCEs == null) { - mergedDictionary._ownerFCEs = new WeakReferenceList(_ownerFCEs.Count); + mergedDictionary._ownerFCEs = new WeakReferenceList(_ownerFCEs.Count); } - foreach (object o in _ownerFCEs) + foreach (FrameworkContentElement fce in _ownerFCEs) { - FrameworkContentElement fce = o as FrameworkContentElement; - if (fce != null) - mergedDictionary.AddOwner(fce); + mergedDictionary.AddOwner(fce); } } @@ -2046,14 +2041,12 @@ private void PropagateParentOwners(ResourceDictionary mergedDictionary) if (mergedDictionary._ownerApps == null) { - mergedDictionary._ownerApps = new WeakReferenceList(_ownerApps.Count); + mergedDictionary._ownerApps = new WeakReferenceList(_ownerApps.Count); } - foreach (object o in _ownerApps) + foreach (Application app in _ownerApps) { - Application app = o as Application; - if (app != null) - mergedDictionary.AddOwner(app); + mergedDictionary.AddOwner(app); } } } @@ -2118,19 +2111,19 @@ private bool ContainsCycle(ResourceDictionary origin) // three properties used by ResourceDictionaryDiagnostics - internal WeakReferenceList FrameworkElementOwners + internal WeakReferenceList FrameworkElementOwners { - get { return _ownerFEs; } + get => _ownerFEs; } - internal WeakReferenceList FrameworkContentElementOwners + internal WeakReferenceList FrameworkContentElementOwners { - get { return _ownerFCEs; } + get => _ownerFCEs; } - internal WeakReferenceList ApplicationOwners + internal WeakReferenceList ApplicationOwners { - get { return _ownerApps; } + get => _ownerApps; } #endregion HelperMethods @@ -2619,11 +2612,12 @@ private enum FallbackState #region Data + private WeakReferenceList _ownerFEs; + private WeakReferenceList _ownerFCEs; + private WeakReferenceList _ownerApps; + private Dictionary> _weakDeferredResourceReferencesMap; + private Hashtable _baseDictionary = null; - private WeakReferenceList _ownerFEs = null; - private WeakReferenceList _ownerFCEs = null; - private WeakReferenceList _ownerApps = null; - private Dictionary _weakDeferredResourceReferencesMap = null; private ObservableCollection _mergedDictionaries = null; private Uri _source = null; private Uri _baseUri = null; diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs index 4650ba7de11..cdb0ba523b3 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/SystemResources.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // @@ -1791,10 +1791,8 @@ internal virtual void RemoveFromDictionary() internal void AddInflatedListener(ResourceReferenceExpression listener) { - if (_inflatedList == null) - { - _inflatedList = new WeakReferenceList(this); - } + _inflatedList ??= new WeakReferenceList(this); + _inflatedList.Add(listener); } @@ -1837,7 +1835,7 @@ internal bool IsInflated private ResourceDictionary _dictionary; protected object _key; protected object _value; - private WeakReferenceList _inflatedList; + private WeakReferenceList _inflatedList; #endregion Data } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs index cc7cc630e1f..bdab2e0785a 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs @@ -357,7 +357,7 @@ public DestroyWindowArgs(IntPtr handle, ushort classAtom) private IntPtr _handle; private UInt16 _classAtom; - private WeakReferenceList _hooks; + private WeakReferenceList _hooks; private int _ownerThreadID; private HwndWrapperHook _wndProc; From 80927a48dc7fe9687364c8db486f66fdce52e900 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 20:14:51 +0200 Subject: [PATCH 11/24] Nullify reference in Dispose for enumerator --- .../src/Shared/MS/Internal/WeakReferenceEnumerator.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs index 4358826aab4..1fe8a146a3e 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs @@ -71,9 +71,10 @@ public void Reset() _strongReference = null; } - public readonly void Dispose() + public void Dispose() { - // This method is here to satisfy the IEnumerator interface. + // Clean up the instance to avoid holding strong references to elements + _strongReference = null; } } From 87be70e7c26d0595e94cea3f2b469ea4526465d8 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 20:48:11 +0200 Subject: [PATCH 12/24] nullability on CopyOnWriteList --- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index b884af53600..9af08a742d4 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#nullable enable + using System.Collections.ObjectModel; namespace MS.Internal; @@ -14,18 +16,14 @@ namespace MS.Internal; /// cache then the list is copied before it is modified and the readonly list is /// released from the cache. /// -internal class CopyOnWriteList +internal class CopyOnWriteList where T : notnull { - private ReadOnlyCollection _readonlyWrapper; + private ReadOnlyCollection? _readonlyWrapper; private List _listList; private readonly object _syncRoot; - public CopyOnWriteList() : this(null) - { - } - - public CopyOnWriteList(object syncRoot) + public CopyOnWriteList(object? syncRoot) { syncRoot ??= new object(); @@ -160,6 +158,7 @@ private int Find(T obj) return i; } } + return -1; } From 4ef74f54fa472fd8ccc474225a99a65dfe93e97d Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 20:48:53 +0200 Subject: [PATCH 13/24] Nullability on WeakReferenceList --- .../src/Shared/MS/Internal/WeakReferenceList.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index 4d9757d7517..20a38c1581a 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#nullable enable + using System.Collections; namespace MS.Internal; @@ -20,7 +22,7 @@ public WeakReferenceList() : base(null) { } - public WeakReferenceList(object syncRoot) : base(syncRoot) + public WeakReferenceList(object? syncRoot) : base(syncRoot) { } @@ -177,7 +179,7 @@ private int FindWeakReference(T obj) for (int i = 0; i < list.Count; i++) { WeakReference weakRef = list[i]; - if (weakRef.TryGetTarget(out T target)) + if (weakRef.TryGetTarget(out T? target)) { if (obj == target) { From d3dc70be5583119da412287e90618396efd837ac Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 20:50:27 +0200 Subject: [PATCH 14/24] Nullability on WeakReferenceList --- .../Shared/MS/Internal/WeakReferenceList.cs | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index 20a38c1581a..cbc890ccc91 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -18,8 +18,15 @@ namespace MS.Internal; /// internal sealed class WeakReferenceList : CopyOnWriteList>, IEnumerable where T : class { - public WeakReferenceList() : base(null) + public int Count { + get + { + lock (SyncRoot) + { + return LiveList.Count; + } + } } public WeakReferenceList(object? syncRoot) : base(syncRoot) @@ -58,19 +65,6 @@ public bool Contains(T item) } } - public int Count - { - get - { - int count = 0; - lock (SyncRoot) - { - count = LiveList.Count; - } - return count; - } - } - /// /// Add a weak reference to the List. /// Returns true if successfully added. From 75a53a0ee0654ec51a054abfde19a4c9e6b05a69 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 20:53:36 +0200 Subject: [PATCH 15/24] Support capacity on WeakReferenceList --- .../System/Windows/ResourceDictionary.cs | 2 +- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 6 ++++++ .../src/Shared/MS/Internal/WeakReferenceList.cs | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs index 510709d0550..357d04ce4d4 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs @@ -1811,7 +1811,7 @@ private WeakReferenceList GetOrCreateWeakReferenceLis if (!_weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences)) { - weakDeferredResourceReferences = new WeakReferenceList(); + weakDeferredResourceReferences = new WeakReferenceList(syncRoot: null); _weakDeferredResourceReferencesMap[resourceKey] = weakDeferredResourceReferences; } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 9af08a742d4..48c9006f381 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -31,6 +31,12 @@ public CopyOnWriteList(object? syncRoot) _listList = new List(); } + public CopyOnWriteList(int capacity) + { + _syncRoot = new object(); + _listList = new List(capacity); + } + /// /// Return a readonly wrapper of the list. Note: this is NOT a copy. /// A non-null _readonlyWrapper is a "Copy on Write" flag. diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index cbc890ccc91..aa5f9aee114 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -29,6 +29,10 @@ public int Count } } + public WeakReferenceList(int capacity) : base(capacity) + { + } + public WeakReferenceList(object? syncRoot) : base(syncRoot) { } From 7255907d812cd939bb4ed38286bdcaafef257eac Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Mon, 26 May 2025 21:07:23 +0200 Subject: [PATCH 16/24] Simplify some checks --- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 4 +- .../Shared/MS/Internal/WeakReferenceList.cs | 46 ++++++++++--------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 48c9006f381..95c5ba057bd 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -16,7 +16,7 @@ namespace MS.Internal; /// cache then the list is copied before it is modified and the readonly list is /// released from the cache. /// -internal class CopyOnWriteList where T : notnull +internal class CopyOnWriteList where T : class { private ReadOnlyCollection? _readonlyWrapper; private List _listList; @@ -159,7 +159,7 @@ private int Find(T obj) // syncRoot Lock MUST be held by the caller. for (int i = 0; i < _listList.Count; i++) { - if (obj.Equals(_listList[i])) + if (obj == _listList[i]) { return i; } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index aa5f9aee114..daf4a8783f1 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -18,6 +18,10 @@ namespace MS.Internal; /// internal sealed class WeakReferenceList : CopyOnWriteList>, IEnumerable where T : class { + /// + /// Retrieves the count of weak references in the list. + /// + /// This can include any dead references currently present in the list. public int Count { get @@ -29,14 +33,27 @@ public int Count } } + /// + /// Creates a new WeakReferenceList with the default capacity. + /// + /// The initial capacity of the list. public WeakReferenceList(int capacity) : base(capacity) { } + /// + /// Creates a new WeakReferenceList with the specified sync root. + /// + /// The synchronization object used to manage access to the list. + /// In case is , a private instance will be created. public WeakReferenceList(object? syncRoot) : base(syncRoot) { } + /// + /// Retrieves a struct-based enumerator for the list. + /// + /// A struct-based enumerator instance. public WeakReferenceListEnumerator GetEnumerator() { return new WeakReferenceListEnumerator(List); @@ -58,14 +75,8 @@ public bool Contains(T item) lock (SyncRoot) { - int index = FindWeakReference(item); - - // If the object is already on the list then - // return true - if (index >= 0) - return true; - - return false; + // If the object is already on the list then return true + return FindWeakReference(item) >= 0; } } @@ -121,12 +132,8 @@ public bool Remove(T obj) { int index = FindWeakReference(obj); - // If the object is not on the list then - // we are done. (return false) - if (index < 0) - return false; - - return RemoveAt(index); + // If the object is not on the list then we are done. + return index >= 0 && RemoveAt(index); } } @@ -143,12 +150,8 @@ public bool Insert(int index, T obj) { int existingIndex = FindWeakReference(obj); - // If the object is already on the list then - // we are done. (return false) - if (existingIndex >= 0) - return false; - - return Internal_Insert(index, new WeakReference(obj)); + // If the object is already on the list then we are done. + return existingIndex < 0 && Internal_Insert(index, new WeakReference(obj)); } } @@ -177,9 +180,10 @@ private int FindWeakReference(T obj) for (int i = 0; i < list.Count; i++) { WeakReference weakRef = list[i]; + if (weakRef.TryGetTarget(out T? target)) { - if (obj == target) + if (target == obj) { foundItem = i; break; From e21e6c21d8e83d4e2d58b9ddc8082cc6677d91dd Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Tue, 27 May 2025 10:48:00 +0200 Subject: [PATCH 17/24] Copy list with pre-allocation manually --- .../Windows/Diagnostics/ResourceDictionaryDiagnostics.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs index 6a5dcecc7bb..66ab9491522 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Diagnostics/ResourceDictionaryDiagnostics.cs @@ -297,8 +297,15 @@ private static IEnumerable GetOwners(WeakReferenceList list) return Array.Empty(); } + // Copy list manually as it doesn't implement ICollection + List owners = new List(list.Count); + foreach (T item in list) + { + owners.Add(item); + } + // Create a read-only copy of the list - return new List(list).AsReadOnly(); + return owners.AsReadOnly(); } #endregion From 2df7f61e04e745cf4982a95f1cbe22fa3a01903b Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Tue, 27 May 2025 11:20:19 +0200 Subject: [PATCH 18/24] Remove casts from NotifyOwners dictionary --- .../System/Windows/ResourceDictionary.cs | 72 ++++++++----------- 1 file changed, 30 insertions(+), 42 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs index 357d04ce4d4..82e4d1037a2 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs @@ -1681,62 +1681,50 @@ private void NotifyOwners(ResourcesChangeInfo info) if (shouldInvalidate || hasImplicitStyles) { // Invalidate all FE owners - if (_ownerFEs != null) + if (_ownerFEs is not null) { - foreach (Object o in _ownerFEs) + foreach (FrameworkElement fe in _ownerFEs) { - FrameworkElement fe = o as FrameworkElement; - if (fe != null) - { - // Set the HasImplicitStyles flag on the owner - if (hasImplicitStyles) - fe.ShouldLookupImplicitStyles = true; - - // If this dictionary has been initialized fire an invalidation - // to let the tree know of this change. - if (shouldInvalidate) - TreeWalkHelper.InvalidateOnResourcesChange(fe, null, info); - } + // Set the HasImplicitStyles flag on the owner + if (hasImplicitStyles) + fe.ShouldLookupImplicitStyles = true; + + // If this dictionary has been initialized fire an invalidation + // to let the tree know of this change. + if (shouldInvalidate) + TreeWalkHelper.InvalidateOnResourcesChange(fe, null, info); } } // Invalidate all FCE owners - if (_ownerFCEs != null) + if (_ownerFCEs is not null) { - foreach (Object o in _ownerFCEs) + foreach (FrameworkContentElement fce in _ownerFCEs) { - FrameworkContentElement fce = o as FrameworkContentElement; - if (fce != null) - { - // Set the HasImplicitStyles flag on the owner - if (hasImplicitStyles) - fce.ShouldLookupImplicitStyles = true; - - // If this dictionary has been initialized fire an invalidation - // to let the tree know of this change. - if (shouldInvalidate) - TreeWalkHelper.InvalidateOnResourcesChange(null, fce, info); - } + // Set the HasImplicitStyles flag on the owner + if (hasImplicitStyles) + fce.ShouldLookupImplicitStyles = true; + + // If this dictionary has been initialized fire an invalidation + // to let the tree know of this change. + if (shouldInvalidate) + TreeWalkHelper.InvalidateOnResourcesChange(null, fce, info); } } // Invalidate all App owners - if (_ownerApps != null) + if (_ownerApps is not null) { - foreach (Object o in _ownerApps) + foreach (Application app in _ownerApps) { - Application app = o as Application; - if (app != null) - { - // Set the HasImplicitStyles flag on the owner - if (hasImplicitStyles) - app.HasImplicitStylesInResources = true; - - // If this dictionary has been initialized fire an invalidation - // to let the tree know of this change. - if (shouldInvalidate) - app.InvalidateResourceReferences(info); - } + // Set the HasImplicitStyles flag on the owner + if (hasImplicitStyles) + app.HasImplicitStylesInResources = true; + + // If this dictionary has been initialized fire an invalidation + // to let the tree know of this change. + if (shouldInvalidate) + app.InvalidateResourceReferences(info); } } } From 29fca9e13cbdd32df7d4a79766fc599b5b214391 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Tue, 27 May 2025 11:25:16 +0200 Subject: [PATCH 19/24] Clean up casts in RemoveParentOwners --- .../System/Windows/ResourceDictionary.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs index 82e4d1037a2..3071dc2acb7 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs @@ -2048,37 +2048,31 @@ private void PropagateParentOwners(ResourceDictionary mergedDictionary) /// internal void RemoveParentOwners(ResourceDictionary mergedDictionary) { - if (_ownerFEs != null) + if (_ownerFEs is not null) { - foreach (Object o in _ownerFEs) + foreach (FrameworkElement fe in _ownerFEs) { - FrameworkElement fe = o as FrameworkElement; mergedDictionary.RemoveOwner(fe); - } } - if (_ownerFCEs != null) + if (_ownerFCEs is not null) { Invariant.Assert(_ownerFCEs.Count > 0); - foreach (Object o in _ownerFCEs) + foreach (FrameworkContentElement fec in _ownerFCEs) { - FrameworkContentElement fec = o as FrameworkContentElement; mergedDictionary.RemoveOwner(fec); - } } - if (_ownerApps != null) + if (_ownerApps is not null) { Invariant.Assert(_ownerApps.Count > 0); - foreach (Object o in _ownerApps) + foreach (Application app in _ownerApps) { - Application app = o as Application; mergedDictionary.RemoveOwner(app); - } } } From eb2653c3990b5dcc41c1f9a2a57630af065d4ead Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Tue, 27 May 2025 12:22:42 +0200 Subject: [PATCH 20/24] Avoid type checks during Contains --- .../System/Windows/ResourceDictionary.cs | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs index 3071dc2acb7..11904b86817 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs @@ -1628,32 +1628,28 @@ internal void RemoveOwner(DispatcherObject owner) RemoveOwnerFromAllMergedDictionaries(owner); } - // Check if the given is an owner to this dictionary - internal bool ContainsOwner(DispatcherObject owner) + /// + /// Checks if the given is owning this dictionary + /// + internal bool ContainsOwner(FrameworkElement owner) { - FrameworkElement fe = owner as FrameworkElement; - if (fe != null) - { - return (_ownerFEs != null && _ownerFEs.Contains(fe)); - } - else - { - FrameworkContentElement fce = owner as FrameworkContentElement; - if (fce != null) - { - return (_ownerFCEs != null && _ownerFCEs.Contains(fce)); - } - else - { - Application app = owner as Application; - if (app != null) - { - return (_ownerApps != null && _ownerApps.Contains(app)); - } - } - } + return _ownerFEs?.Contains(owner) ?? false; + } - return false; + /// + /// Checks if the given is owning this dictionary + /// + internal bool ContainsOwner(FrameworkContentElement owner) + { + return _ownerFCEs?.Contains(owner) ?? false; + } + + /// + /// Checks if the given is owning this dictionary + /// + internal bool ContainsOwner(Application owner) + { + return _ownerApps?.Contains(owner) ?? false; } // Helper method that tries to set IsInitialized to true if BeginInit hasn't been called before this. From 7575d11e701f4bdc98e2570d498d7ea704fed030 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Tue, 27 May 2025 15:20:39 +0200 Subject: [PATCH 21/24] Fix up some documentation --- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 102 ++++++++++-------- .../MS/Internal/WeakReferenceEnumerator.cs | 17 +-- 2 files changed, 66 insertions(+), 53 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 95c5ba057bd..98c642df627 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -4,25 +4,41 @@ #nullable enable using System.Collections.ObjectModel; +using System.Runtime.CompilerServices; namespace MS.Internal; /// -/// This is a ThreadSafe ArrayList that uses Copy On Write to support consistency. -/// - When the "List" property is requested a readonly reference to the -/// list is returned and a reference to the readonly list is cached. -/// - If the "List" is requested again, the same cached reference is returned. -/// - When the list is modified, if a readonly reference is present in the -/// cache then the list is copied before it is modified and the readonly list is -/// released from the cache. +/// This is a ThreadSafe List that uses Copy On Write to support consistency. +/// - When the "List" property is requested a readonly reference to the +/// list is returned and a reference to the readonly list is cached. +/// - If the "List" is requested again, the same cached reference is returned. +/// - When the list is modified, if a readonly reference is present in the +/// cache then the list is copied before it is modified and the readonly list is +/// released from the cache. /// -internal class CopyOnWriteList where T : class +internal abstract class CopyOnWriteList where T : class { private ReadOnlyCollection? _readonlyWrapper; private List _listList; private readonly object _syncRoot; + /// + /// Creates a new WeakReferenceList with the default capacity. + /// + /// The initial capacity of the list. + public CopyOnWriteList(int capacity) + { + _syncRoot = new object(); + _listList = new List(capacity); + } + + /// + /// Creates a new CopyOnWriteList with the specified sync root. + /// + /// The synchronization object used to manage access to the list. + /// In case is , a private instance will be created. public CopyOnWriteList(object? syncRoot) { syncRoot ??= new object(); @@ -31,20 +47,14 @@ public CopyOnWriteList(object? syncRoot) _listList = new List(); } - public CopyOnWriteList(int capacity) - { - _syncRoot = new object(); - _listList = new List(capacity); - } - /// - /// Return a readonly wrapper of the list. Note: this is NOT a copy. - /// A non-null _readonlyWrapper is a "Copy on Write" flag. - /// Methods that change the list (eg. Add() and Remove()) are - /// responsible for: - /// 1) Checking _readonlyWrapper and copying the list before modifying it. - /// 2) Clearing _readonlyWrapper. + /// Return a readonly wrapper of the list. + /// A non-null _readonlyWrapper is a "Copy on Write" flag. + /// Methods that change the list (eg. Add() and Remove()) are responsible for: + /// 1) Checking _readonlyWrapper and copying the list before modifying it. + /// 2) Clearing _readonlyWrapper. /// + /// Note: This is NOT a copy. public ReadOnlyCollection List { get @@ -63,11 +73,11 @@ public ReadOnlyCollection List } /// - /// Add an object to the List. - /// Returns true if successfully added. - /// Returns false if object is already on the list. + /// Add an object to the List. + /// Returns true if successfully added. + /// Returns false if object is already on the list. /// - public virtual bool Add(T obj) + protected bool Add(T obj) { Debug.Assert(obj is not null, "CopyOnWriteList.Add() should not be passed null."); @@ -83,11 +93,11 @@ public virtual bool Add(T obj) } /// - /// Remove an object from the List. - /// Returns true if successfully removed. - /// Returns false if object was not on the list. + /// Remove an object from the List. + /// Returns true if successfully removed. + /// Returns false if object was not on the list. /// - public virtual bool Remove(T obj) + protected bool Remove(T obj) { Debug.Assert(obj is not null, "CopyOnWriteList.Remove() should not be passed null."); lock (_syncRoot) @@ -104,8 +114,8 @@ public virtual bool Remove(T obj) } /// - /// This allows derived classes to take the lock. This is mostly used - /// to extend Add() and Remove() etc. + /// This allows derived classes to take the lock. This is mostly used + /// to extend Add() and Remove() etc. /// protected object SyncRoot { @@ -113,10 +123,10 @@ protected object SyncRoot } /// - /// This is protected and the caller can get into real serious trouble - /// using this. Because this points at the real current list without - /// any copy on write protection. So the caller must really know what - /// they are doing. + /// This is protected and the caller can get into real serious trouble + /// using this. Because this points at the real current list without + /// any copy on write protection. So the caller must really know what + /// they are doing. /// protected List LiveList { @@ -124,9 +134,9 @@ protected List LiveList } /// - /// Add an object to the List. - /// Without any error checks. - /// For use by derived classes that implement there own error checks. + /// Add an object to the List. + /// Without any error checks. + /// For use by derived classes that implement their own error checks. /// protected bool Internal_Add(T obj) { @@ -138,9 +148,9 @@ protected bool Internal_Add(T obj) } /// - /// Insert an object into the List at the given index. - /// Without any error checks. - /// For use by derived classes that implement there own error checks. + /// Insert an object into the List at the given index. + /// Without any error checks. + /// For use by derived classes that implement there own error checks. /// protected bool Internal_Insert(int index, T obj) { @@ -152,7 +162,7 @@ protected bool Internal_Insert(int index, T obj) } /// - /// Find an object on the List. + /// Find an object on the List. /// private int Find(T obj) { @@ -169,12 +179,11 @@ private int Find(T obj) } /// - /// Remove the object at a given index from the List. - /// Returns true if successfully removed. - /// Returns false if index is outside the range of the list. - /// - /// This is protected because it operates on the LiveList + /// Remove the object at a given index from the List. + /// Returns true if successfully removed. + /// Returns false if index is outside the range of the list. /// + /// This is protected because it operates on the . protected bool RemoveAt(int index) { // syncRoot Lock MUST be held by the caller. @@ -188,6 +197,7 @@ protected bool RemoveAt(int index) return true; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] protected void DoCopyOnWriteCheck() { // syncRoot Lock MUST be held by the caller. diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs index 1fe8a146a3e..6ba7a5be587 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceEnumerator.cs @@ -9,15 +9,18 @@ namespace MS.Internal; /// -/// This allows callers to "foreach" through a WeakReferenceList. -/// Each weakreference is checked for liveness and "current" -/// actually returns a strong reference to the current element. +/// This allows callers to "foreach" through a WeakReferenceList. +/// Each weak reference is checked for liveness and "current" +/// actually returns a strong reference to the current element. /// /// -/// Due to the way enumerators function, this enumerator often -/// holds a cached strong reference to the "Current" element. -/// This should not be a problem unless the caller stops enumerating -/// before the end of the list AND holds the enumerator alive forever. +/// +/// Due to the way enumerators function, this enumerator often +/// holds a cached strong reference to the "Current" element. +/// This should not be a problem unless the caller stops enumerating +/// before the end of the list AND holds the enumerator alive forever. +/// +/// Explicitly calling Dispose, this removes the strong reference. /// internal struct WeakReferenceListEnumerator : IEnumerator where T : class { From 9294376c5ee77acbba27fe8c65ca982e1ccb2c94 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Tue, 27 May 2025 15:50:57 +0200 Subject: [PATCH 22/24] Fix build --- src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs index bdab2e0785a..87a9b95aeac 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs @@ -220,13 +220,13 @@ private void Dispose(bool disposing, bool isHwndBeingDestroyed) public void AddHook(HwndWrapperHook hook) { - _hooks ??= []; + _hooks ??= new WeakReferenceList(syncRoot: null); _hooks.Insert(0, hook); } internal void AddHookLast(HwndWrapperHook hook) { - _hooks ??= []; + _hooks ??= new WeakReferenceList(syncRoot: null); _hooks.Add(hook); } From 1d5a7e0badabb42ea509cb1b9809ca993ee2df08 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Tue, 27 May 2025 18:29:28 +0200 Subject: [PATCH 23/24] Avoid casts during enumeration --- .../System/Windows/ResourceDictionary.cs | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs index 11904b86817..7093caf50c5 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ResourceDictionary.cs @@ -1816,7 +1816,6 @@ internal void RemoveDeferredResourceReference(DeferredResourceReference deferred /// private void ValidateDeferredResourceReferences(object resourceKey) { - if (_weakDeferredResourceReferencesMap is null) { return; @@ -1824,36 +1823,29 @@ private void ValidateDeferredResourceReferences(object resourceKey) if (resourceKey is null) { - foreach (var weakDeferredResourceReferences in _weakDeferredResourceReferencesMap.Values) + foreach (WeakReferenceList weakReferencesList in _weakDeferredResourceReferencesMap.Values) { - foreach (var weakResourceReference in weakDeferredResourceReferences) + foreach (DeferredResourceReference weakReference in weakReferencesList) { - DeferredResourceReference deferredResourceReference = weakResourceReference as DeferredResourceReference; - - Inflate(deferredResourceReference); + Inflate(weakReference); } } } else { - if (_weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out var weakDeferredResourceReferences)) + if (_weakDeferredResourceReferencesMap.TryGetValue(resourceKey, out WeakReferenceList weakReferencesList)) { - foreach (var weakResourceReference in weakDeferredResourceReferences) + foreach (DeferredResourceReference weakReference in weakReferencesList) { - DeferredResourceReference deferredResourceReference = weakResourceReference as DeferredResourceReference; - - Inflate(deferredResourceReference); + Inflate(weakReference); } } } - return; - - void Inflate(DeferredResourceReference deferredResourceReference) + static void Inflate(DeferredResourceReference deferredResourceReference) { - // This will inflate the deferred reference, causing it - // to be removed from the list. The list may also be - // purged of dead references. + // This will inflate the deferred reference, causing it to be removed from the list. + // The list may also be purged of dead references. deferredResourceReference?.GetValue(BaseValueSourceInternal.Unknown); } } From 3b3696689bd39657ae01c04bb1725879cfcbdf32 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Tue, 27 May 2025 18:50:50 +0200 Subject: [PATCH 24/24] Fix naming and simplify checks --- .../src/Shared/MS/Internal/CopyOnWriteList.cs | 50 ++++++++----------- .../Shared/MS/Internal/WeakReferenceList.cs | 2 +- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs index 98c642df627..0b5ecd01719 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/CopyOnWriteList.cs @@ -9,7 +9,7 @@ namespace MS.Internal; /// -/// This is a ThreadSafe List that uses Copy On Write to support consistency. +/// This is a ThreadSafe that uses Copy On Write to support consistency. /// - When the "List" property is requested a readonly reference to the /// list is returned and a reference to the readonly list is cached. /// - If the "List" is requested again, the same cached reference is returned. @@ -20,7 +20,7 @@ namespace MS.Internal; internal abstract class CopyOnWriteList where T : class { private ReadOnlyCollection? _readonlyWrapper; - private List _listList; + private List _liveList; private readonly object _syncRoot; @@ -31,7 +31,7 @@ internal abstract class CopyOnWriteList where T : class public CopyOnWriteList(int capacity) { _syncRoot = new object(); - _listList = new List(capacity); + _liveList = new List(capacity); } /// @@ -44,7 +44,7 @@ public CopyOnWriteList(object? syncRoot) syncRoot ??= new object(); _syncRoot = syncRoot; - _listList = new List(); + _liveList = new List(); } /// @@ -63,7 +63,7 @@ public ReadOnlyCollection List lock (_syncRoot) { - _readonlyWrapper ??= _listList.AsReadOnly(); + _readonlyWrapper ??= _liveList.AsReadOnly(); tempList = _readonlyWrapper; } @@ -85,10 +85,7 @@ protected bool Add(T obj) { int index = Find(obj); - if (index >= 0) - return false; - - return Internal_Add(obj); + return index < 0 && Internal_Add(obj); } } @@ -104,12 +101,8 @@ protected bool Remove(T obj) { int index = Find(obj); - // If the object is not on the list then - // we are done. (return false) - if (index < 0) - return false; - - return RemoveAt(index); + // If the object is not on the list then we are done. + return index >= 0 && RemoveAt(index); } } @@ -119,7 +112,7 @@ protected bool Remove(T obj) /// protected object SyncRoot { - get { return _syncRoot; } + get => _syncRoot; } /// @@ -130,7 +123,7 @@ protected object SyncRoot /// protected List LiveList { - get { return _listList; } + get => _liveList; } /// @@ -142,7 +135,7 @@ protected bool Internal_Add(T obj) { DoCopyOnWriteCheck(); - _listList.Add(obj); + _liveList.Add(obj); return true; } @@ -156,7 +149,7 @@ protected bool Internal_Insert(int index, T obj) { DoCopyOnWriteCheck(); - _listList.Insert(index, obj); + _liveList.Insert(index, obj); return true; } @@ -167,9 +160,9 @@ protected bool Internal_Insert(int index, T obj) private int Find(T obj) { // syncRoot Lock MUST be held by the caller. - for (int i = 0; i < _listList.Count; i++) + for (int i = 0; i < _liveList.Count; i++) { - if (obj == _listList[i]) + if (obj == _liveList[i]) { return i; } @@ -187,26 +180,27 @@ private int Find(T obj) protected bool RemoveAt(int index) { // syncRoot Lock MUST be held by the caller. - if (index < 0 || index >= _listList.Count) + if ((uint)index >= (uint)_liveList.Count) return false; DoCopyOnWriteCheck(); - _listList.RemoveAt(index); + _liveList.RemoveAt(index); return true; } + /// + /// This must be called under lock held. + /// [MethodImpl(MethodImplOptions.AggressiveInlining)] protected void DoCopyOnWriteCheck() { - // syncRoot Lock MUST be held by the caller. - // If we have exposed (given out) a readonly reference to this - // version of the list, then clone a new internal copy and cut - // the old version free. + // If we have exposed (given out) a readonly reference to this version of the list, + // then clone a new internal copy and cut the old version free. if (_readonlyWrapper is not null) { - _listList = new List(_listList); + _liveList = new List(_liveList); _readonlyWrapper = null; } } diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs index daf4a8783f1..58ee2b80a9e 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/WeakReferenceList.cs @@ -8,7 +8,7 @@ namespace MS.Internal; /// -/// This is a Cached ThreadSafe ArrayList of WeakReferences. +/// This is a Cached ThreadSafe of WeakReferences. /// - When the "List" property is requested a readonly reference to the /// list is returned and a reference to the readonly list is cached. /// - If the "List" is requested again, the same cached reference is returned.