Skip to content

Commit

Permalink
Replace WeakReference with WeakReference<T> (dotnet#1141)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Wraith2
Wraith2 authored and Davoud Eshtehari committed Aug 31, 2021
1 parent 269c80b commit 08250aa
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 147 deletions.
57 changes: 18 additions & 39 deletions ...ata.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,52 +14,37 @@ internal abstract class DbReferenceCollection
private struct CollectionEntry
{
private int _tag; // information about the reference
private WeakReference _weak; // the reference itself.
private WeakReference<object> _weak; // the reference itself.

public void NewTarget(int tag, object target)
{
Debug.Assert(!HasTarget, "Entry already has a valid target");
Debug.Assert(!TryGetTarget(out object _) , "Entry already has a valid target");
Debug.Assert(tag != 0, "Bad tag");
Debug.Assert(target != null, "Invalid target");

if (_weak == null)
{
_weak = new WeakReference(target, false);
_weak = new WeakReference<object>(target, false);
}
else
{
_weak.Target = target;
_weak.SetTarget(target);
}
_tag = tag;
}

public void RemoveTarget()
{
_tag = 0;
_weak.SetTarget(null);
}

public bool HasTarget
{
get
{
return ((_tag != 0) && (_weak.IsAlive));
}
}

public int Tag
{
get
{
return _tag;
}
}
public int Tag => _tag;

public object Target
public bool TryGetTarget(out object target)
{
get
{
return (_tag == 0 ? null : _weak.Target);
}
target = null;
return _tag != 0 && _weak.TryGetTarget(out target);
}
}

Expand Down Expand Up @@ -94,7 +79,7 @@ protected void AddItem(object value, int tag)
if (_items[i].Tag == 0)
{
_items[i].NewTarget(tag, value);
Debug.Assert(_items[i].HasTarget, "missing expected target");
Debug.Assert(_items[i].TryGetTarget(out object _), "missing expected target");
itemAdded = true;
break;
}
Expand All @@ -113,10 +98,10 @@ protected void AddItem(object value, int tag)
{
for (int i = 0; i <= _lastItemIndex; ++i)
{
if (!_items[i].HasTarget)
if (!_items[i].TryGetTarget(out object _))
{
_items[i].NewTarget(tag, value);
Debug.Assert(_items[i].HasTarget, "missing expected target");
Debug.Assert(_items[i].TryGetTarget(out object _), "missing expected target");
itemAdded = true;
break;
}
Expand Down Expand Up @@ -145,20 +130,15 @@ internal T FindItem<T>(int tag, Func<T, bool> filterMethod) where T : class
{
if (_optimisticCount > 0)
{
// Loop through the items
for (int counter = 0; counter <= _lastItemIndex; counter++)
{
// Check tag (should be easiest and quickest)
if (_items[counter].Tag == tag)
{
// NOTE: Check if the returned value is null twice may seem wasteful, but this if for performance
// Since checking for null twice is cheaper than calling both HasTarget and Target OR always attempting to typecast
object value = _items[counter].Target;
if (value != null)
if (_items[counter].TryGetTarget(out object value))
{
// Make sure the item has the correct type and passes the filtering
T tempItem = value as T;
if ((tempItem != null) && (filterMethod(tempItem)))
if (value is T tempItem && filterMethod(tempItem))
{
return tempItem;
}
Expand Down Expand Up @@ -194,13 +174,12 @@ public void Notify(int message)
{
for (int index = 0; index <= _lastItemIndex; ++index)
{
object value = _items[index].Target; // checks tag & gets target
if (null != value)
if (_items[index].TryGetTarget(out object value))
{
NotifyItem(message, _items[index].Tag, value);
_items[index].RemoveTarget();
}
Debug.Assert(!_items[index].HasTarget, "Unexpected target after notifying");
Debug.Assert(!_items[index].TryGetTarget(out object _), "Unexpected target after notifying");
}
_optimisticCount = 0;
}
Expand Down Expand Up @@ -244,8 +223,8 @@ protected void RemoveItem(object value)
{
for (int index = 0; index <= _lastItemIndex; ++index)
{
if (value == _items[index].Target)
{ // checks tag & gets target
if (_items[index].TryGetTarget(out object target) && value == target)
{
_items[index].RemoveTarget();
_optimisticCount--;
break;
Expand Down
28 changes: 12 additions & 16 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ sealed internal class SqlInternalTransaction
private int _openResultCount; // passed in the MARS headers
private SqlInternalConnection _innerConnection;
private bool _disposing; // used to prevent us from throwing exceptions while we're disposing
private WeakReference _parent; // weak ref to the outer transaction object; needs to be weak to allow GC to occur.
private WeakReference<SqlTransaction> _parent; // weak ref to the outer transaction object; needs to be weak to allow GC to occur.

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

if (null != outerTransaction)
{
_parent = new WeakReference(outerTransaction);
_parent = new WeakReference<SqlTransaction>(outerTransaction);
}

_transactionId = transactionId;
Expand Down Expand Up @@ -157,14 +157,14 @@ internal bool IsOrphaned
Debug.Assert(_transactionType == TransactionType.LocalFromTSQL, "invalid state");
result = false;
}
else if (null == _parent.Target)
else if (!_parent.TryGetTarget(out SqlTransaction _))
{
// We have an parent, but parent was GC'ed.
// We had a parent, but parent was GC'ed.
result = true;
}
else
{
// We have an parent, and parent is alive.
// We have a parent, and parent is alive.
result = false;
}

Expand Down Expand Up @@ -203,9 +203,9 @@ internal SqlTransaction Parent
SqlTransaction result = null;
// Should we protect against this, since this probably is an invalid state?
Debug.Assert(null != _parent, "Why are we calling Parent with no parent?");
if (null != _parent)
if (_parent != null && _parent.TryGetTarget(out SqlTransaction target))
{
result = (SqlTransaction)_parent.Target;
result = target;
}
return result;
}
Expand Down Expand Up @@ -385,7 +385,7 @@ internal int IncrementAndObtainOpenResultCount()
internal void InitParent(SqlTransaction transaction)
{
Debug.Assert(_parent == null, "Why do we have a parent on InitParent?");
_parent = new WeakReference(transaction);
_parent = new WeakReference<SqlTransaction>(transaction);
}

internal void Rollback()
Expand Down Expand Up @@ -533,20 +533,16 @@ internal void Zombie()

private void ZombieParent()
{
if (null != _parent)
if (_parent != null && _parent.TryGetTarget(out SqlTransaction parent))
{
SqlTransaction parent = (SqlTransaction)_parent.Target;
if (null != parent)
{
parent.Zombie();
}
_parent = null;
parent.Zombie();
}
_parent = null;
}

internal string TraceString()
{
return String.Format(/*IFormatProvider*/ null, "(ObjId={0}, tranId={1}, state={2}, type={3}, open={4}, disp={5}",
return string.Format(/*IFormatProvider*/ null, "(ObjId={0}, tranId={1}, state={2}, type={3}, open={4}, disp={5}",
ObjectID, _transactionId, _transactionState, _transactionType, _openResultCount, _disposing);
}
}
Expand Down
30 changes: 12 additions & 18 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public TimeoutState(int value)


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

Expand Down Expand Up @@ -383,40 +383,34 @@ internal bool IsOrphaned
{
get
{
Debug.Assert((0 == _activateCount && !_owner.IsAlive) // in pool
|| (1 == _activateCount && _owner.IsAlive && _owner.Target != null)
|| (1 == _activateCount && !_owner.IsAlive), "Unknown state on TdsParserStateObject.IsOrphaned!");
return (0 != _activateCount && !_owner.IsAlive);
bool isAlive = _owner.TryGetTarget(out object target);
Debug.Assert((0 == _activateCount && !isAlive) // in pool
|| (1 == _activateCount && isAlive && target != null)
|| (1 == _activateCount && !isAlive), "Unknown state on TdsParserStateObject.IsOrphaned!");
return (0 != _activateCount && !isAlive);
}
}

internal object Owner
{
set
{
Debug.Assert(value == null || !_owner.IsAlive || ((value is SqlDataReader) && (((SqlDataReader)value).Command == _owner.Target)), "Should not be changing the owner of an owned stateObj");
SqlDataReader reader = value as SqlDataReader;
if (reader == null)
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");
if (value is SqlDataReader reader)
{
_readerState = null;
_readerState = reader._sharedState;
}
else
{
_readerState = reader._sharedState;
_readerState = null;
}
_owner.Target = value;
_owner.SetTarget(value);
}
}

internal abstract uint DisableSsl();

internal bool HasOwner
{
get
{
return _owner.IsAlive;
}
}
internal bool HasOwner => _owner.TryGetTarget(out object _);

internal TdsParser Parser
{
Expand Down
Loading

0 comments on commit 08250aa

Please sign in to comment.