[release/7.0] Remove locks from COM events delegate management. (#76034)

* Remove locks from COM events delegate management.

This removes locks and instead assumes the collection is immutable.

* Remove uses of var.

* Use array instead of List<T>

* Revert "Use array instead of List<T>"

This reverts commit 5ba348f.

* Revert "Revert "Use array instead of List<T>""

This reverts commit aa3dddd.

* Remove usage of `Delegate.Combine`. Upon deeper inspection there doesn't seem to be any value to using that mechanism. An added bonus is that it simplifies the logic.

* Use some sugar

* Simplify the RemoveAll scenario.

* Style nits.

Co-authored-by: Aaron Robinson <[email protected]>

Author: github-actions[bot]

src/libraries/Common/src/System/Runtime/InteropServices/ComEventsMethod.cs

@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Threading;
 using System.Reflection;
 
 namespace System.Runtime.InteropServices
@@ -99,7 +100,7 @@ private void PreProcessSignature()
         /// Since multicast delegate's built-in chaining supports only chaining instances of the same type,
         /// we need to complement this design by using an explicit linked list data structure.
         /// </summary>
-        private readonly List<DelegateWrapper> _delegateWrappers = new List<DelegateWrapper>();
+        private DelegateWrapper[] _delegateWrappers = Array.Empty<DelegateWrapper>();
 
         private readonly int _dispid;
         private ComEventsMethod? _next;
@@ -154,48 +155,36 @@ public static ComEventsMethod Add(ComEventsMethod? methods, ComEventsMethod meth
 
         public bool Empty
         {
-            get
-            {
-                lock (_delegateWrappers)
-                {
-                    return _delegateWrappers.Count == 0;
-                }
-            }
+            get => _delegateWrappers.Length == 0;
         }
 
         public void AddDelegate(Delegate d, bool wrapArgs = false)
         {
-            lock (_delegateWrappers)
+            DelegateWrapper[] wrappers, newWrappers;
+            do
             {
-                // Update an existing delegate wrapper
-                foreach (DelegateWrapper wrapper in _delegateWrappers)
-                {
-                    if (wrapper.Delegate.GetType() == d.GetType() && wrapper.WrapArgs == wrapArgs)
-                    {
-                        wrapper.Delegate = Delegate.Combine(wrapper.Delegate, d);
-                        return;
-                    }
-                }
-
-                var newWrapper = new DelegateWrapper(d, wrapArgs);
-                _delegateWrappers.Add(newWrapper);
-            }
+                wrappers = _delegateWrappers;
+                newWrappers = new DelegateWrapper[wrappers.Length + 1];
+                wrappers.CopyTo(newWrappers, 0);
+                newWrappers[^1] = new DelegateWrapper(d, wrapArgs);
+            } while (!PublishNewWrappers(newWrappers, wrappers));
         }
 
         public void RemoveDelegate(Delegate d, bool wrapArgs = false)
         {
-            lock (_delegateWrappers)
+            DelegateWrapper[] wrappers, newWrappers;
+            do
             {
+                wrappers = _delegateWrappers;
+
                 // Find delegate wrapper index
                 int removeIdx = -1;
-                DelegateWrapper? wrapper = null;
-                for (int i = 0; i < _delegateWrappers.Count; i++)
+                for (int i = 0; i < wrappers.Length; i++)
                 {
-                    DelegateWrapper wrapperMaybe = _delegateWrappers[i];
-                    if (wrapperMaybe.Delegate.GetType() == d.GetType() && wrapperMaybe.WrapArgs == wrapArgs)
+                    DelegateWrapper wrapperMaybe = wrappers[i];
+                    if (wrapperMaybe.Delegate == d && wrapperMaybe.WrapArgs == wrapArgs)
                     {
                         removeIdx = i;
-                        wrapper = wrapperMaybe;
                         break;
                     }
                 }
@@ -206,67 +195,42 @@ public void RemoveDelegate(Delegate d, bool wrapArgs = false)
                     return;
                 }
 
-                // Update wrapper or remove from collection
-                Delegate? newDelegate = Delegate.Remove(wrapper!.Delegate, d);
-                if (newDelegate != null)
-                {
-                    wrapper.Delegate = newDelegate;
-                }
-                else
-                {
-                    _delegateWrappers.RemoveAt(removeIdx);
-                }
-            }
+                newWrappers = new DelegateWrapper[wrappers.Length - 1];
+                wrappers.AsSpan(0, removeIdx).CopyTo(newWrappers);
+                wrappers.AsSpan(removeIdx + 1).CopyTo(newWrappers.AsSpan(removeIdx));
+            } while (!PublishNewWrappers(newWrappers, wrappers));
         }
 
         public void RemoveDelegates(Func<Delegate, bool> condition)
         {
-            lock (_delegateWrappers)
+            DelegateWrapper[] wrappers, newWrappers;
+            do
             {
-                // Find delegate wrapper indexes. Iterate in reverse such that the list to remove is sorted by high to low index.
-                List<int> toRemove = new List<int>();
-                for (int i = _delegateWrappers.Count - 1; i >= 0; i--)
-                {
-                    DelegateWrapper wrapper = _delegateWrappers[i];
-                    Delegate[] invocationList = wrapper.Delegate.GetInvocationList();
-                    foreach (Delegate delegateMaybe in invocationList)
-                    {
-                        if (condition(delegateMaybe))
-                        {
-                            Delegate? newDelegate = Delegate.Remove(wrapper!.Delegate, delegateMaybe);
-                            if (newDelegate != null)
-                            {
-                                wrapper.Delegate = newDelegate;
-                            }
-                            else
-                            {
-                                toRemove.Add(i);
-                            }
-                        }
-                    }
-                }
-
-                foreach (int idx in toRemove)
-                {
-                    _delegateWrappers.RemoveAt(idx);
-                }
+                wrappers = _delegateWrappers;
+                List<DelegateWrapper> tmp = new(wrappers);
+                tmp.RemoveAll(w => condition(w.Delegate));
+                newWrappers = tmp.ToArray();
             }
+            while (!PublishNewWrappers(newWrappers, wrappers));
         }
 
         public object? Invoke(object[] args)
         {
             Debug.Assert(!Empty);
             object? result = null;
 
-            lock (_delegateWrappers)
+            foreach (DelegateWrapper wrapper in _delegateWrappers)
             {
-                foreach (DelegateWrapper wrapper in _delegateWrappers)
-                {
-                    result = wrapper.Invoke(args);
-                }
+                result = wrapper.Invoke(args);
             }
 
             return result;
         }
+
+        // Attempt to update the member wrapper field
+        private bool PublishNewWrappers(DelegateWrapper[] newWrappers, DelegateWrapper[] currentMaybe)
+        {
+            return Interlocked.CompareExchange(ref _delegateWrappers, newWrappers, currentMaybe) == currentMaybe;
+        }
     }
 }