diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs b/src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs index 958dedb4e5..e7efdad252 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs @@ -14,21 +14,21 @@ internal abstract class DbReferenceCollection private struct CollectionEntry { private int _tag; // information about the reference - private WeakReference _weak; // the reference itself. + private WeakReference _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(target, false); } else { - _weak.Target = target; + _weak.SetTarget(target); } _tag = tag; } @@ -36,30 +36,15 @@ public void NewTarget(int tag, object target) 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); } } @@ -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; } @@ -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; } @@ -145,20 +130,15 @@ internal T FindItem(int tag, Func 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; } @@ -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; } @@ -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; diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs index a5d1b072d6..aeb37cda27 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs @@ -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 _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); @@ -58,7 +58,7 @@ internal SqlInternalTransaction(SqlInternalConnection innerConnection, Transacti if (null != outerTransaction) { - _parent = new WeakReference(outerTransaction); + _parent = new WeakReference(outerTransaction); } _transactionId = transactionId; @@ -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; } @@ -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; } @@ -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(transaction); } internal void Rollback() @@ -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); } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 8645cee07e..8f07484efa 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -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 _owner = new WeakReference(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 @@ -375,10 +375,11 @@ 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); } } @@ -386,29 +387,22 @@ 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 { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs index 0cd2c8289d..1e69bcaba9 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbReferenceCollection.cs @@ -15,21 +15,21 @@ internal abstract class DbReferenceCollection private struct CollectionEntry { private int _tag; // information about the reference - private WeakReference _weak; // the reference itself. + private WeakReference _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(target, false); } else { - _weak.Target = target; + _weak.SetTarget(target); } _tag = tag; } @@ -37,30 +37,15 @@ public void NewTarget(int tag, object target) 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); } } @@ -95,7 +80,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; } @@ -114,10 +99,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; } @@ -152,14 +137,10 @@ internal T FindItem(int tag, Func filterMethod) where T : class // 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; } @@ -195,13 +176,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; } @@ -245,8 +225,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; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 131b22088c..8a6be2314e 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -63,7 +63,7 @@ internal int ObjectID private readonly TdsParser _parser; // TdsParser pointer private SNIHandle _sessionHandle = null; // the SNI handle we're to work on - private readonly WeakReference _owner = new WeakReference(null); // the owner of this session, used to track when it's been orphaned + private readonly WeakReference _owner = new WeakReference(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 @@ -395,10 +395,11 @@ 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; } } @@ -406,27 +407,20 @@ 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 bool HasOwner - { - get - { - return _owner.IsAlive; - } - } + internal bool HasOwner => _owner.TryGetTarget(out object _); internal TdsParser Parser { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/sqlinternaltransaction.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/sqlinternaltransaction.cs index e7581774c9..7438dab671 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/sqlinternaltransaction.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/sqlinternaltransaction.cs @@ -26,11 +26,10 @@ internal enum TransactionType Delegated = 3, Distributed = 4, Context = 5, // only valid in proc. - }; + } sealed internal class SqlInternalTransaction { - internal const long NullTransactionId = 0; private TransactionState _transactionState; @@ -39,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 _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 = System.Threading.Interlocked.Increment(ref _objectTypeCount); @@ -60,7 +59,7 @@ internal SqlInternalTransaction(SqlInternalConnection innerConnection, Transacti if (null != outerTransaction) { - _parent = new WeakReference(outerTransaction); + _parent = new WeakReference(outerTransaction); } _transactionId = transactionId; @@ -168,14 +167,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; } @@ -214,9 +213,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; } @@ -420,7 +419,7 @@ internal Int32 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(transaction); } internal void Rollback() @@ -591,20 +590,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); } }