Skip to content

Commit 98ea349

Browse files
Wraith2Wraith2
authored
Replace WeakReference with WeakReference<T> (#1141)
1 parent d1bac50 commit 98ea349

File tree

6 files changed

+85
-147
lines changed

6 files changed

+85
-147
lines changed

‎src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs

+18-39
Original file line numberDiff line numberDiff line change
@@ -14,52 +14,37 @@ internal abstract class DbReferenceCollection
1414
private struct CollectionEntry
1515
{
1616
private int _tag; // information about the reference
17-
private WeakReference _weak; // the reference itself.
17+
private WeakReference<object> _weak; // the reference itself.
1818

1919
public void NewTarget(int tag, object target)
2020
{
21-
Debug.Assert(!HasTarget, "Entry already has a valid target");
21+
Debug.Assert(!TryGetTarget(out object _) , "Entry already has a valid target");
2222
Debug.Assert(tag != 0, "Bad tag");
2323
Debug.Assert(target != null, "Invalid target");
2424

2525
if (_weak == null)
2626
{
27-
_weak = new WeakReference(target, false);
27+
_weak = new WeakReference<object>(target, false);
2828
}
2929
else
3030
{
31-
_weak.Target = target;
31+
_weak.SetTarget(target);
3232
}
3333
_tag = tag;
3434
}
3535

3636
public void RemoveTarget()
3737
{
3838
_tag = 0;
39+
_weak.SetTarget(null);
3940
}
4041

41-
public bool HasTarget
42-
{
43-
get
44-
{
45-
return ((_tag != 0) && (_weak.IsAlive));
46-
}
47-
}
48-
49-
public int Tag
50-
{
51-
get
52-
{
53-
return _tag;
54-
}
55-
}
42+
public int Tag => _tag;
5643

57-
public object Target
44+
public bool TryGetTarget(out object target)
5845
{
59-
get
60-
{
61-
return (_tag == 0 ? null : _weak.Target);
62-
}
46+
target = null;
47+
return _tag != 0 && _weak.TryGetTarget(out target);
6348
}
6449
}
6550

@@ -94,7 +79,7 @@ protected void AddItem(object value, int tag)
9479
if (_items[i].Tag == 0)
9580
{
9681
_items[i].NewTarget(tag, value);
97-
Debug.Assert(_items[i].HasTarget, "missing expected target");
82+
Debug.Assert(_items[i].TryGetTarget(out object _), "missing expected target");
9883
itemAdded = true;
9984
break;
10085
}
@@ -113,10 +98,10 @@ protected void AddItem(object value, int tag)
11398
{
11499
for (int i = 0; i <= _lastItemIndex; ++i)
115100
{
116-
if (!_items[i].HasTarget)
101+
if (!_items[i].TryGetTarget(out object _))
117102
{
118103
_items[i].NewTarget(tag, value);
119-
Debug.Assert(_items[i].HasTarget, "missing expected target");
104+
Debug.Assert(_items[i].TryGetTarget(out object _), "missing expected target");
120105
itemAdded = true;
121106
break;
122107
}
@@ -145,20 +130,15 @@ internal T FindItem<T>(int tag, Func<T, bool> filterMethod) where T : class
145130
{
146131
if (_optimisticCount > 0)
147132
{
148-
// Loop through the items
149133
for (int counter = 0; counter <= _lastItemIndex; counter++)
150134
{
151135
// Check tag (should be easiest and quickest)
152136
if (_items[counter].Tag == tag)
153137
{
154-
// NOTE: Check if the returned value is null twice may seem wasteful, but this if for performance
155-
// Since checking for null twice is cheaper than calling both HasTarget and Target OR always attempting to typecast
156-
object value = _items[counter].Target;
157-
if (value != null)
138+
if (_items[counter].TryGetTarget(out object value))
158139
{
159140
// Make sure the item has the correct type and passes the filtering
160-
T tempItem = value as T;
161-
if ((tempItem != null) && (filterMethod(tempItem)))
141+
if (value is T tempItem && filterMethod(tempItem))
162142
{
163143
return tempItem;
164144
}
@@ -194,13 +174,12 @@ public void Notify(int message)
194174
{
195175
for (int index = 0; index <= _lastItemIndex; ++index)
196176
{
197-
object value = _items[index].Target; // checks tag & gets target
198-
if (null != value)
177+
if (_items[index].TryGetTarget(out object value))
199178
{
200179
NotifyItem(message, _items[index].Tag, value);
201180
_items[index].RemoveTarget();
202181
}
203-
Debug.Assert(!_items[index].HasTarget, "Unexpected target after notifying");
182+
Debug.Assert(!_items[index].TryGetTarget(out object _), "Unexpected target after notifying");
204183
}
205184
_optimisticCount = 0;
206185
}
@@ -244,8 +223,8 @@ protected void RemoveItem(object value)
244223
{
245224
for (int index = 0; index <= _lastItemIndex; ++index)
246225
{
247-
if (value == _items[index].Target)
248-
{ // checks tag & gets target
226+
if (_items[index].TryGetTarget(out object target) && value == target)
227+
{
249228
_items[index].RemoveTarget();
250229
_optimisticCount--;
251230
break;

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs

+12-16
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ sealed internal class SqlInternalTransaction
3838
private int _openResultCount; // passed in the MARS headers
3939
private SqlInternalConnection _innerConnection;
4040
private bool _disposing; // used to prevent us from throwing exceptions while we're disposing
41-
private WeakReference _parent; // weak ref to the outer transaction object; needs to be weak to allow GC to occur.
41+
private WeakReference<SqlTransaction> _parent; // weak ref to the outer transaction object; needs to be weak to allow GC to occur.
4242

4343
private static int _objectTypeCount; // EventSource counter
4444
internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount);
@@ -58,7 +58,7 @@ internal SqlInternalTransaction(SqlInternalConnection innerConnection, Transacti
5858

5959
if (null != outerTransaction)
6060
{
61-
_parent = new WeakReference(outerTransaction);
61+
_parent = new WeakReference<SqlTransaction>(outerTransaction);
6262
}
6363

6464
_transactionId = transactionId;
@@ -157,14 +157,14 @@ internal bool IsOrphaned
157157
Debug.Assert(_transactionType == TransactionType.LocalFromTSQL, "invalid state");
158158
result = false;
159159
}
160-
else if (null == _parent.Target)
160+
else if (!_parent.TryGetTarget(out SqlTransaction _))
161161
{
162-
// We have an parent, but parent was GC'ed.
162+
// We had a parent, but parent was GC'ed.
163163
result = true;
164164
}
165165
else
166166
{
167-
// We have an parent, and parent is alive.
167+
// We have a parent, and parent is alive.
168168
result = false;
169169
}
170170

@@ -203,9 +203,9 @@ internal SqlTransaction Parent
203203
SqlTransaction result = null;
204204
// Should we protect against this, since this probably is an invalid state?
205205
Debug.Assert(null != _parent, "Why are we calling Parent with no parent?");
206-
if (null != _parent)
206+
if (_parent != null && _parent.TryGetTarget(out SqlTransaction target))
207207
{
208-
result = (SqlTransaction)_parent.Target;
208+
result = target;
209209
}
210210
return result;
211211
}
@@ -385,7 +385,7 @@ internal int IncrementAndObtainOpenResultCount()
385385
internal void InitParent(SqlTransaction transaction)
386386
{
387387
Debug.Assert(_parent == null, "Why do we have a parent on InitParent?");
388-
_parent = new WeakReference(transaction);
388+
_parent = new WeakReference<SqlTransaction>(transaction);
389389
}
390390

391391
internal void Rollback()
@@ -533,20 +533,16 @@ internal void Zombie()
533533

534534
private void ZombieParent()
535535
{
536-
if (null != _parent)
536+
if (_parent != null && _parent.TryGetTarget(out SqlTransaction parent))
537537
{
538-
SqlTransaction parent = (SqlTransaction)_parent.Target;
539-
if (null != parent)
540-
{
541-
parent.Zombie();
542-
}
543-
_parent = null;
538+
parent.Zombie();
544539
}
540+
_parent = null;
545541
}
546542

547543
internal string TraceString()
548544
{
549-
return String.Format(/*IFormatProvider*/ null, "(ObjId={0}, tranId={1}, state={2}, type={3}, open={4}, disp={5}",
545+
return string.Format(/*IFormatProvider*/ null, "(ObjId={0}, tranId={1}, state={2}, type={3}, open={4}, disp={5}",
550546
ObjectID, _transactionId, _transactionState, _transactionType, _openResultCount, _disposing);
551547
}
552548
}

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs

+12-18
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public TimeoutState(int value)
6666

6767

6868
protected readonly TdsParser _parser; // TdsParser pointer
69-
private readonly WeakReference _owner = new WeakReference(null); // the owner of this session, used to track when it's been orphaned
69+
private readonly WeakReference<object> _owner = new WeakReference<object>(null); // the owner of this session, used to track when it's been orphaned
7070
internal SqlDataReader.SharedState _readerState; // susbset of SqlDataReader state (if it is the owner) necessary for parsing abandoned results in TDS
7171
private int _activateCount; // 0 when we're in the pool, 1 when we're not, all others are an error
7272

@@ -375,40 +375,34 @@ internal bool IsOrphaned
375375
{
376376
get
377377
{
378-
Debug.Assert((0 == _activateCount && !_owner.IsAlive) // in pool
379-
|| (1 == _activateCount && _owner.IsAlive && _owner.Target != null)
380-
|| (1 == _activateCount && !_owner.IsAlive), "Unknown state on TdsParserStateObject.IsOrphaned!");
381-
return (0 != _activateCount && !_owner.IsAlive);
378+
bool isAlive = _owner.TryGetTarget(out object target);
379+
Debug.Assert((0 == _activateCount && !isAlive) // in pool
380+
|| (1 == _activateCount && isAlive && target != null)
381+
|| (1 == _activateCount && !isAlive), "Unknown state on TdsParserStateObject.IsOrphaned!");
382+
return (0 != _activateCount && !isAlive);
382383
}
383384
}
384385

385386
internal object Owner
386387
{
387388
set
388389
{
389-
Debug.Assert(value == null || !_owner.IsAlive || ((value is SqlDataReader) && (((SqlDataReader)value).Command == _owner.Target)), "Should not be changing the owner of an owned stateObj");
390-
SqlDataReader reader = value as SqlDataReader;
391-
if (reader == null)
390+
Debug.Assert(value == null || !_owner.TryGetTarget(out object target) || value is SqlDataReader reader1 && reader1.Command == target, "Should not be changing the owner of an owned stateObj");
391+
if (value is SqlDataReader reader)
392392
{
393-
_readerState = null;
393+
_readerState = reader._sharedState;
394394
}
395395
else
396396
{
397-
_readerState = reader._sharedState;
397+
_readerState = null;
398398
}
399-
_owner.Target = value;
399+
_owner.SetTarget(value);
400400
}
401401
}
402402

403403
internal abstract uint DisableSsl();
404404

405-
internal bool HasOwner
406-
{
407-
get
408-
{
409-
return _owner.IsAlive;
410-
}
411-
}
405+
internal bool HasOwner => _owner.TryGetTarget(out object _);
412406

413407
internal TdsParser Parser
414408
{

0 commit comments

Comments
 (0)