Skip to content

Commit e4d115a

Browse files
Remove locks from COM events delegate management. (#75863)
* 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.
1 parent b4ac094 commit e4d115a

File tree

1 file changed

+36
-72
lines changed

1 file changed

+36
-72
lines changed

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

Lines changed: 36 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Collections.Generic;
55
using System.Diagnostics;
6+
using System.Threading;
67
using System.Reflection;
78

89
namespace System.Runtime.InteropServices
@@ -99,7 +100,7 @@ private void PreProcessSignature()
99100
/// Since multicast delegate's built-in chaining supports only chaining instances of the same type,
100101
/// we need to complement this design by using an explicit linked list data structure.
101102
/// </summary>
102-
private readonly List<DelegateWrapper> _delegateWrappers = new List<DelegateWrapper>();
103+
private DelegateWrapper[] _delegateWrappers = Array.Empty<DelegateWrapper>();
103104

104105
private readonly int _dispid;
105106
private ComEventsMethod? _next;
@@ -154,48 +155,36 @@ public static ComEventsMethod Add(ComEventsMethod? methods, ComEventsMethod meth
154155

155156
public bool Empty
156157
{
157-
get
158-
{
159-
lock (_delegateWrappers)
160-
{
161-
return _delegateWrappers.Count == 0;
162-
}
163-
}
158+
get => _delegateWrappers.Length == 0;
164159
}
165160

166161
public void AddDelegate(Delegate d, bool wrapArgs = false)
167162
{
168-
lock (_delegateWrappers)
163+
DelegateWrapper[] wrappers, newWrappers;
164+
do
169165
{
170-
// Update an existing delegate wrapper
171-
foreach (DelegateWrapper wrapper in _delegateWrappers)
172-
{
173-
if (wrapper.Delegate.GetType() == d.GetType() && wrapper.WrapArgs == wrapArgs)
174-
{
175-
wrapper.Delegate = Delegate.Combine(wrapper.Delegate, d);
176-
return;
177-
}
178-
}
179-
180-
var newWrapper = new DelegateWrapper(d, wrapArgs);
181-
_delegateWrappers.Add(newWrapper);
182-
}
166+
wrappers = _delegateWrappers;
167+
newWrappers = new DelegateWrapper[wrappers.Length + 1];
168+
wrappers.CopyTo(newWrappers, 0);
169+
newWrappers[^1] = new DelegateWrapper(d, wrapArgs);
170+
} while (!PublishNewWrappers(newWrappers, wrappers));
183171
}
184172

185173
public void RemoveDelegate(Delegate d, bool wrapArgs = false)
186174
{
187-
lock (_delegateWrappers)
175+
DelegateWrapper[] wrappers, newWrappers;
176+
do
188177
{
178+
wrappers = _delegateWrappers;
179+
189180
// Find delegate wrapper index
190181
int removeIdx = -1;
191-
DelegateWrapper? wrapper = null;
192-
for (int i = 0; i < _delegateWrappers.Count; i++)
182+
for (int i = 0; i < wrappers.Length; i++)
193183
{
194-
DelegateWrapper wrapperMaybe = _delegateWrappers[i];
195-
if (wrapperMaybe.Delegate.GetType() == d.GetType() && wrapperMaybe.WrapArgs == wrapArgs)
184+
DelegateWrapper wrapperMaybe = wrappers[i];
185+
if (wrapperMaybe.Delegate == d && wrapperMaybe.WrapArgs == wrapArgs)
196186
{
197187
removeIdx = i;
198-
wrapper = wrapperMaybe;
199188
break;
200189
}
201190
}
@@ -206,67 +195,42 @@ public void RemoveDelegate(Delegate d, bool wrapArgs = false)
206195
return;
207196
}
208197

209-
// Update wrapper or remove from collection
210-
Delegate? newDelegate = Delegate.Remove(wrapper!.Delegate, d);
211-
if (newDelegate != null)
212-
{
213-
wrapper.Delegate = newDelegate;
214-
}
215-
else
216-
{
217-
_delegateWrappers.RemoveAt(removeIdx);
218-
}
219-
}
198+
newWrappers = new DelegateWrapper[wrappers.Length - 1];
199+
wrappers.AsSpan(0, removeIdx).CopyTo(newWrappers);
200+
wrappers.AsSpan(removeIdx + 1).CopyTo(newWrappers.AsSpan(removeIdx));
201+
} while (!PublishNewWrappers(newWrappers, wrappers));
220202
}
221203

222204
public void RemoveDelegates(Func<Delegate, bool> condition)
223205
{
224-
lock (_delegateWrappers)
206+
DelegateWrapper[] wrappers, newWrappers;
207+
do
225208
{
226-
// Find delegate wrapper indexes. Iterate in reverse such that the list to remove is sorted by high to low index.
227-
List<int> toRemove = new List<int>();
228-
for (int i = _delegateWrappers.Count - 1; i >= 0; i--)
229-
{
230-
DelegateWrapper wrapper = _delegateWrappers[i];
231-
Delegate[] invocationList = wrapper.Delegate.GetInvocationList();
232-
foreach (Delegate delegateMaybe in invocationList)
233-
{
234-
if (condition(delegateMaybe))
235-
{
236-
Delegate? newDelegate = Delegate.Remove(wrapper!.Delegate, delegateMaybe);
237-
if (newDelegate != null)
238-
{
239-
wrapper.Delegate = newDelegate;
240-
}
241-
else
242-
{
243-
toRemove.Add(i);
244-
}
245-
}
246-
}
247-
}
248-
249-
foreach (int idx in toRemove)
250-
{
251-
_delegateWrappers.RemoveAt(idx);
252-
}
209+
wrappers = _delegateWrappers;
210+
List<DelegateWrapper> tmp = new(wrappers);
211+
tmp.RemoveAll(w => condition(w.Delegate));
212+
newWrappers = tmp.ToArray();
253213
}
214+
while (!PublishNewWrappers(newWrappers, wrappers));
254215
}
255216

256217
public object? Invoke(object[] args)
257218
{
258219
Debug.Assert(!Empty);
259220
object? result = null;
260221

261-
lock (_delegateWrappers)
222+
foreach (DelegateWrapper wrapper in _delegateWrappers)
262223
{
263-
foreach (DelegateWrapper wrapper in _delegateWrappers)
264-
{
265-
result = wrapper.Invoke(args);
266-
}
224+
result = wrapper.Invoke(args);
267225
}
268226

269227
return result;
270228
}
229+
230+
// Attempt to update the member wrapper field
231+
private bool PublishNewWrappers(DelegateWrapper[] newWrappers, DelegateWrapper[] currentMaybe)
232+
{
233+
return Interlocked.CompareExchange(ref _delegateWrappers, newWrappers, currentMaybe) == currentMaybe;
234+
}
271235
}
272236
}

0 commit comments

Comments
 (0)