Skip to content

Commit

Permalink
Remove locks from COM events delegate management. (dotnet#75863)
Browse files Browse the repository at this point in the history
* Remove locks from COM events delegate management.

This removes locks and instead assumes the collection is immutable.

* Use array instead of List<T>

* Remove usage of `Delegate.Combine`. Upon deeper inspection there
doesn't seem to be any value to using that mechanism.
  • Loading branch information
AaronRobinsonMSFT committed Sep 22, 2022
1 parent b4ac094 commit e4d115a
Showing 1 changed file with 36 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Diagnostics;
using System.Threading;
using System.Reflection;

namespace System.Runtime.InteropServices
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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;
}
}
}

0 comments on commit e4d115a

Please sign in to comment.