Skip to content

Commit e912151

Browse files
stephentoubstephentoub
authored
Replace non-generic WeakReference usage in several libraries (#48618)
* Replace non-generic WeakReference usage in System.Private.Runtime.InteropServices.JavaScript * Replace non-generic WeakReference usage in System.Private.Xml * Replace non-generic WeakReference usage in System.Diagnostics.TraceSource * Address PR feedback
1 parent dd6bc8c commit e912151

File tree

7 files changed

+76
-90
lines changed

7 files changed

+76
-90
lines changed

‎src/libraries/System.Diagnostics.TraceSource/src/System/Diagnostics/Switch.cs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public abstract class Switch
2727
private readonly string? _defaultValue;
2828
private object? _initializedLock;
2929

30-
private static readonly List<WeakReference> s_switches = new List<WeakReference>();
30+
private static readonly List<WeakReference<Switch>> s_switches = new List<WeakReference<Switch>>();
3131
private static int s_LastCollectionCount;
3232
private StringDictionary? _attributes;
3333

@@ -66,7 +66,7 @@ protected Switch(string displayName, string? description, string? defaultSwitchV
6666
lock (s_switches)
6767
{
6868
_pruneCachedSwitches();
69-
s_switches.Add(new WeakReference(this));
69+
s_switches.Add(new WeakReference<Switch>(this));
7070
}
7171

7272
_defaultValue = defaultSwitchValue;
@@ -78,11 +78,10 @@ private static void _pruneCachedSwitches()
7878
{
7979
if (s_LastCollectionCount != GC.CollectionCount(2))
8080
{
81-
List<WeakReference> buffer = new List<WeakReference>(s_switches.Count);
81+
List<WeakReference<Switch>> buffer = new List<WeakReference<Switch>>(s_switches.Count);
8282
for (int i = 0; i < s_switches.Count; i++)
8383
{
84-
Switch? s = ((Switch?)s_switches[i].Target);
85-
if (s != null)
84+
if (s_switches[i].TryGetTarget(out _))
8685
{
8786
buffer.Add(s_switches[i]);
8887
}
@@ -236,8 +235,7 @@ internal static void RefreshAll()
236235
_pruneCachedSwitches();
237236
for (int i = 0; i < s_switches.Count; i++)
238237
{
239-
Switch? swtch = ((Switch?)s_switches[i].Target);
240-
if (swtch != null)
238+
if (s_switches[i].TryGetTarget(out Switch? swtch))
241239
{
242240
swtch.Refresh();
243241
}

‎src/libraries/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceSource.cs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace System.Diagnostics
1111
{
1212
public class TraceSource
1313
{
14-
private static readonly List<WeakReference> s_tracesources = new List<WeakReference>();
14+
private static readonly List<WeakReference<TraceSource>> s_tracesources = new List<WeakReference<TraceSource>>();
1515
private static int s_LastCollectionCount;
1616

1717
private volatile SourceSwitch? _internalSwitch;
@@ -40,7 +40,7 @@ public TraceSource(string name, SourceLevels defaultLevel)
4040
lock (s_tracesources)
4141
{
4242
_pruneCachedTraceSources();
43-
s_tracesources.Add(new WeakReference(this));
43+
s_tracesources.Add(new WeakReference<TraceSource>(this));
4444
}
4545
}
4646

@@ -50,11 +50,10 @@ private static void _pruneCachedTraceSources()
5050
{
5151
if (s_LastCollectionCount != GC.CollectionCount(2))
5252
{
53-
List<WeakReference> buffer = new List<WeakReference>(s_tracesources.Count);
53+
List<WeakReference<TraceSource>> buffer = new List<WeakReference<TraceSource>>(s_tracesources.Count);
5454
for (int i = 0; i < s_tracesources.Count; i++)
5555
{
56-
TraceSource? tracesource = ((TraceSource?)s_tracesources[i].Target);
57-
if (tracesource != null)
56+
if (s_tracesources[i].TryGetTarget(out _))
5857
{
5958
buffer.Add(s_tracesources[i]);
6059
}
@@ -153,8 +152,7 @@ internal static void RefreshAll()
153152
_pruneCachedTraceSources();
154153
for (int i = 0; i < s_tracesources.Count; i++)
155154
{
156-
TraceSource? tracesource = ((TraceSource?)s_tracesources[i].Target);
157-
if (tracesource != null)
155+
if (s_tracesources[i].TryGetTarget(out TraceSource? tracesource))
158156
{
159157
tracesource.Refresh();
160158
}

‎src/libraries/System.Private.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSObject.cs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ public class JSObject : AnyRef, IJSObject, IDisposable
2020
{
2121
internal object? RawObject;
2222

23-
// Right now this is used for wrapping Delegates
24-
private WeakReference? WeakRawObject;
23+
private WeakReference<Delegate>? WeakRawObject;
2524

2625
// to detect redundant calls
2726
public bool IsDisposed { get; private set; }
@@ -47,7 +46,7 @@ internal JSObject(int jsHandle, object rawObj) : base(jsHandle, false)
4746

4847
internal JSObject(int jsHandle, Delegate rawDelegate, bool ownsHandle = true) : base(jsHandle, ownsHandle)
4948
{
50-
WeakRawObject = new WeakReference(rawDelegate, false);
49+
WeakRawObject = new WeakReference<Delegate>(rawDelegate, trackResurrection: false);
5150
}
5251

5352
/// <summary>
@@ -149,11 +148,11 @@ public int Length
149148
/// <param name="prop">The String name or Symbol of the property to test.</param>
150149
public bool PropertyIsEnumerable(string prop) => (bool)Invoke("propertyIsEnumerable", prop);
151150

152-
internal bool IsWeakWrapper => WeakRawObject?.Target != null;
151+
internal bool IsWeakWrapper => WeakRawObject?.TryGetTarget(out _) == true;
153152

154153
internal object? GetWrappedObject()
155154
{
156-
return RawObject ?? WeakRawObject?.Target;
155+
return RawObject ?? (WeakRawObject is WeakReference<Delegate> wr && wr.TryGetTarget(out Delegate? d) ? d : null);
157156
}
158157
internal void FreeHandle()
159158
{

‎src/libraries/System.Private.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace System.Runtime.InteropServices.JavaScript
1212
{
1313
public static class Runtime
1414
{
15-
private static readonly Dictionary<int, WeakReference> _boundObjects = new Dictionary<int, WeakReference>();
15+
private static readonly Dictionary<int, WeakReference<JSObject>> _boundObjects = new Dictionary<int, WeakReference<JSObject>>();
1616
private static readonly Dictionary<object, JSObject> _rawToJS = new Dictionary<object, JSObject>();
1717
// _weakDelegateTable is a ConditionalWeakTable with the Delegate and associated JSObject:
1818
// Key Lifetime:
@@ -77,49 +77,46 @@ public static void DumpAotProfileData (ref byte buf, int len, string extraArg)
7777

7878
public static int BindJSObject(int jsId, bool ownsHandle, int mappedType)
7979
{
80-
WeakReference? reference;
80+
JSObject? target = null;
81+
8182
lock (_boundObjects)
8283
{
83-
if (_boundObjects.TryGetValue(jsId, out reference))
84-
{
85-
if ((reference.Target == null) || ((reference.Target as JSObject)?.IsDisposed == true))
86-
{
87-
_boundObjects.Remove(jsId);
88-
reference = null;
89-
}
90-
}
91-
92-
if (reference == null)
84+
if (!_boundObjects.TryGetValue(jsId, out WeakReference<JSObject>? reference) ||
85+
!reference.TryGetTarget(out target) ||
86+
target.IsDisposed)
9387
{
9488
IntPtr jsIntPtr = (IntPtr)jsId;
95-
reference = new WeakReference(mappedType > 0 ? BindJSType(jsIntPtr, ownsHandle, mappedType) : new JSObject(jsIntPtr, ownsHandle), true);
96-
_boundObjects.Add(jsId, reference);
89+
target = mappedType > 0 ? BindJSType(jsIntPtr, ownsHandle, mappedType) : new JSObject(jsIntPtr, ownsHandle);
90+
_boundObjects[jsId] = new WeakReference<JSObject>(target, trackResurrection: true);
9791
}
9892
}
99-
return reference.Target is JSObject target ? target.Int32Handle : 0;
93+
94+
return target.Int32Handle;
10095
}
10196

10297
public static int BindCoreCLRObject(int jsId, int gcHandle)
10398
{
10499
GCHandle h = (GCHandle)(IntPtr)gcHandle;
105-
JSObject? obj;
100+
JSObject? obj = null;
106101

107102
lock (_boundObjects)
108103
{
109-
if (_boundObjects.TryGetValue(jsId, out WeakReference? existingObj))
104+
if (_boundObjects.TryGetValue(jsId, out WeakReference<JSObject>? wr))
110105
{
111-
var instance = existingObj.Target as JSObject;
112-
if (instance?.Int32Handle != (int)(IntPtr)h && h.IsAllocated)
106+
if (!wr.TryGetTarget(out JSObject? instance) || (instance.Int32Handle != (int)(IntPtr)h && h.IsAllocated))
107+
{
113108
throw new JSException(SR.Format(SR.MultipleHandlesPointingJsId, jsId));
109+
}
114110

115111
obj = instance;
116112
}
117-
else
113+
else if (h.Target is JSObject instance)
118114
{
119-
obj = h.Target as JSObject;
120-
_boundObjects.Add(jsId, new WeakReference(obj, true));
115+
_boundObjects.Add(jsId, new WeakReference<JSObject>(instance, trackResurrection: true));
116+
obj = instance;
121117
}
122118
}
119+
123120
return obj?.Int32Handle ?? 0;
124121
}
125122

@@ -199,7 +196,7 @@ public static int BindExistingObject(object rawObj, int jsId)
199196
jsObject = new JSObject(jsId, dele);
200197
lock (_boundObjects)
201198
{
202-
_boundObjects.Add(jsId, new WeakReference(jsObject));
199+
_boundObjects.Add(jsId, new WeakReference<JSObject>(jsObject));
203200
}
204201
lock (_weakDelegateTable)
205202
{
@@ -486,10 +483,11 @@ public static void SafeHandleReleaseByHandle(int jsId)
486483
#endif
487484
lock (_boundObjects)
488485
{
489-
if (_boundObjects.TryGetValue(jsId, out WeakReference? reference))
486+
if (_boundObjects.TryGetValue(jsId, out WeakReference<JSObject>? reference))
490487
{
491-
Debug.Assert(reference.Target != null, $"\tSafeHandleReleaseByHandle: did not find active target {jsId} / target: {reference.Target}");
492-
SafeHandleRelease((AnyRef)reference.Target);
488+
reference.TryGetTarget(out JSObject? target);
489+
Debug.Assert(target != null, $"\tSafeHandleReleaseByHandle: did not find active target {jsId}");
490+
SafeHandleRelease(target);
493491
}
494492
else
495493
{

‎src/libraries/System.Private.Xml.Linq/src/System/Xml/Linq/XNamespace.cs‎

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ public sealed class XNamespace
1616
internal const string xmlPrefixNamespace = "http://www.w3.org/XML/1998/namespace";
1717
internal const string xmlnsPrefixNamespace = "http://www.w3.org/2000/xmlns/";
1818

19-
private static XHashtable<WeakReference>? s_namespaces;
20-
private static WeakReference? s_refNone;
21-
private static WeakReference? s_refXml;
22-
private static WeakReference? s_refXmlns;
19+
private static XHashtable<WeakReference<XNamespace>>? s_namespaces;
20+
private static WeakReference<XNamespace>? s_refNone;
21+
private static WeakReference<XNamespace>? s_refXml;
22+
private static WeakReference<XNamespace>? s_refXmlns;
2323

2424
private readonly string _namespaceName;
2525
private readonly int _hashCode;
@@ -236,9 +236,9 @@ internal static XNamespace Get(string namespaceName, int index, int count)
236236

237237
// Use CompareExchange to ensure that exactly one XHashtable<WeakReference> is used to store namespaces
238238
if (s_namespaces == null)
239-
Interlocked.CompareExchange(ref s_namespaces, new XHashtable<WeakReference>(ExtractNamespace, NamespacesCapacity), null);
239+
Interlocked.CompareExchange(ref s_namespaces, new XHashtable<WeakReference<XNamespace>>(ExtractNamespace, NamespacesCapacity), null);
240240

241-
WeakReference? refNamespace;
241+
WeakReference<XNamespace>? refNamespace;
242242
XNamespace? ns;
243243

244244
// Keep looping until a non-null namespace has been retrieved
@@ -252,10 +252,10 @@ internal static XNamespace Get(string namespaceName, int index, int count)
252252
if (count == xmlnsPrefixNamespace.Length && string.CompareOrdinal(namespaceName, index, xmlnsPrefixNamespace, 0, count) == 0) return Xmlns;
253253

254254
// Go ahead and create the namespace and add it to the table
255-
refNamespace = s_namespaces.Add(new WeakReference(new XNamespace(namespaceName.Substring(index, count))));
255+
refNamespace = s_namespaces.Add(new WeakReference<XNamespace>(new XNamespace(namespaceName.Substring(index, count))));
256256
}
257257

258-
ns = (refNamespace != null) ? (XNamespace?)refNamespace.Target : null;
258+
ns = refNamespace != null && refNamespace.TryGetTarget(out XNamespace? target) ? target : null;
259259
}
260260
while (ns == null);
261261

@@ -273,45 +273,38 @@ private static string ExtractLocalName(XName n)
273273
}
274274

275275
/// <summary>
276-
/// This function is used by the <see cref="XHashtable{WeakReference}"/> to extract the XNamespace that the WeakReference is
277-
/// referencing. In cases where the XNamespace has been cleaned up, this function returns null.
276+
/// This function is used to extract the XNamespace that the WeakReference is referencing.
277+
/// In cases where the XNamespace has been cleaned up, this function returns null.
278278
/// </summary>
279-
private static string? ExtractNamespace(WeakReference? r)
280-
{
281-
XNamespace? ns;
282-
283-
if (r == null || (ns = (XNamespace?)r.Target) == null)
284-
return null;
285-
286-
return ns.NamespaceName;
287-
}
279+
private static string? ExtractNamespace(WeakReference<XNamespace>? r) =>
280+
r is not null &&
281+
r.TryGetTarget(out XNamespace? target) ? target.NamespaceName : null;
288282

289283
/// <summary>
290284
/// Ensure that an XNamespace object for 'namespaceName' has been atomically created. In other words, all outstanding
291285
/// references to this particular namespace, on any thread, must all be to the same object. Care must be taken,
292286
/// since other threads can be concurrently calling this method, and the target of a WeakReference can be cleaned up
293287
/// at any time by the GC.
294288
/// </summary>
295-
private static XNamespace EnsureNamespace(ref WeakReference? refNmsp, string namespaceName)
289+
private static XNamespace EnsureNamespace(ref WeakReference<XNamespace>? refNmsp, string namespaceName)
296290
{
297-
WeakReference? refOld;
291+
WeakReference<XNamespace>? refOld;
298292

299293
// Keep looping until a non-null namespace has been retrieved
300294
while (true)
301295
{
302296
// Save refNmsp in local variable, so we can work on a value that will not be changed by another thread
303297
refOld = refNmsp;
304298

305-
if (refOld != null)
299+
if (refOld != null && refOld.TryGetTarget(out XNamespace? ns))
306300
{
307301
// If the target of the WeakReference is non-null, then we're done--just return the value
308-
XNamespace? ns = (XNamespace?)refOld.Target;
309-
if (ns != null) return ns;
302+
return ns;
310303
}
311304

312305
// Either refNmsp is null, or its target is null, so update it
313306
// Make sure to do this atomically, so that we can guarantee atomicity of XNamespace objects
314-
Interlocked.CompareExchange(ref refNmsp, new WeakReference(new XNamespace(namespaceName)), refOld);
307+
Interlocked.CompareExchange(ref refNmsp, new WeakReference<XNamespace>(new XNamespace(namespaceName)), refOld);
315308
}
316309
}
317310
}

‎src/libraries/System.Private.Xml/src/System/Xml/Dom/XmlDocument.cs‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -284,23 +284,27 @@ internal bool AddIdInfo(XmlName eleName, XmlName attrName)
284284
return GetIDInfoByElement_(eleName);
285285
}
286286

287-
private WeakReference? GetElement(ArrayList elementList, XmlElement elem)
287+
private WeakReference<XmlElement>? GetElement(ArrayList elementList, XmlElement elem)
288288
{
289289
ArrayList gcElemRefs = new ArrayList();
290-
foreach (WeakReference elemRef in elementList)
290+
foreach (WeakReference<XmlElement> elemRef in elementList)
291291
{
292-
if (!elemRef.IsAlive)
292+
if (!elemRef.TryGetTarget(out XmlElement? target))
293+
{
293294
//take notes on the garbage collected nodes
294295
gcElemRefs.Add(elemRef);
296+
}
295297
else
296298
{
297-
if ((XmlElement?)(elemRef.Target) == elem)
299+
if (target == elem)
298300
return elemRef;
299301
}
300302
}
303+
301304
//Clear out the gced elements
302-
foreach (WeakReference elemRef in gcElemRefs)
305+
foreach (WeakReference<XmlElement> elemRef in gcElemRefs)
303306
elementList.Remove(elemRef);
307+
304308
return null;
305309
}
306310

@@ -311,15 +315,15 @@ internal void AddElementWithId(string id, XmlElement elem)
311315
if (_htElementIdMap == null)
312316
_htElementIdMap = new Hashtable();
313317
ArrayList elementList = new ArrayList();
314-
elementList.Add(new WeakReference(elem));
318+
elementList.Add(new WeakReference<XmlElement>(elem));
315319
_htElementIdMap.Add(id, elementList);
316320
}
317321
else
318322
{
319323
// there are other element(s) that has the same id
320324
ArrayList elementList = (ArrayList)(_htElementIdMap[id]!);
321325
if (GetElement(elementList, elem) == null)
322-
elementList.Add(new WeakReference(elem));
326+
elementList.Add(new WeakReference<XmlElement>(elem));
323327
}
324328
}
325329

@@ -328,7 +332,7 @@ internal void RemoveElementWithId(string id, XmlElement elem)
328332
if (_htElementIdMap != null && _htElementIdMap.Contains(id))
329333
{
330334
ArrayList elementList = (ArrayList)(_htElementIdMap[id]!);
331-
WeakReference? elemRef = GetElement(elementList, elem);
335+
WeakReference<XmlElement>? elemRef = GetElement(elementList, elem);
332336
if (elemRef != null)
333337
{
334338
elementList.Remove(elemRef);
@@ -951,11 +955,9 @@ public virtual XmlNodeList GetElementsByTagName(string localName, string namespa
951955
ArrayList? elementList = (ArrayList?)(_htElementIdMap[elementId]);
952956
if (elementList != null)
953957
{
954-
foreach (WeakReference elemRef in elementList)
958+
foreach (WeakReference<XmlElement> elemRef in elementList)
955959
{
956-
XmlElement? elem = (XmlElement?)elemRef.Target;
957-
if (elem != null
958-
&& elem.IsConnected())
960+
if (elemRef.TryGetTarget(out XmlElement? elem) && elem.IsConnected())
959961
return elem;
960962
}
961963
}

0 commit comments

Comments
 (0)