Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace WeakReference with WeakReference<T> #1141

Merged
merged 2 commits into from
Jul 5, 2021
Merged

Replace WeakReference with WeakReference<T> #1141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8286827
replace WeakReference with WeakReference<T>
Wraith2 Jun 27, 2021
2d07931
unconditionally null weakreference
Wraith2 Jul 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 18 additions & 39 deletions ...ata.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs
View file
Open in desktop
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
Open in desktop
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))
Wraith2 marked this conversation as resolved.
Show resolved Hide resolved
{
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
Open in desktop
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 @@ -375,40 +375,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