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

Change WeakReference to WeakReference<T> #1122

Merged
merged 1 commit into from
Jun 21, 2021
Merged

Change WeakReference to WeakReference<T> #1122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 19 additions & 17 deletions ...Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal abstract partial class DbConnectionInternal
private readonly bool _hidePassword;
private readonly ConnectionState _state;

private readonly WeakReference _owningObject = new WeakReference(null, false); // [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections)
private readonly WeakReference<DbConnection> _owningObject = new WeakReference<DbConnection>(null, false); // [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections)

private DbConnectionPool _connectionPool; // the pooler that the connection came from (Pooled connections only)
private DbReferenceCollection _referenceCollection; // collection of objects that we need to notify in some way when we're being deactivated
Expand Down Expand Up @@ -63,8 +63,7 @@ internal bool CanBePooled
{
get
{
bool flag = (!_connectionIsDoomed && !_cannotBePooled && !_owningObject.IsAlive);
return flag;
return (!_connectionIsDoomed && !_cannotBePooled && !_owningObject.TryGetTarget(out DbConnection _));
}
}

Expand Down Expand Up @@ -103,8 +102,7 @@ internal bool IsEmancipated
// of the pool and it's owning object is no longer around to
// return it.

bool value = (_pooledCount < 1) && !_owningObject.IsAlive;
return value;
return (_pooledCount < 1) && !_owningObject.TryGetTarget(out DbConnection _);
}
}

Expand All @@ -118,13 +116,17 @@ internal bool IsInPool
}


protected internal object Owner
protected internal DbConnection Owner
{
// We use a weak reference to the owning object so we can identify when
// it has been garbage collected without throwing exceptions.
get
{
return _owningObject.Target;
if (_owningObject.TryGetTarget(out DbConnection connection))
{
return connection;
}
return null;
}
}

Expand Down Expand Up @@ -276,13 +278,13 @@ protected internal virtual DataTable GetSchema(DbConnectionFactory factory, DbCo
return metaDataFactory.GetSchema(outerConnection, collectionName, restrictions);
}

internal void MakeNonPooledObject(object owningObject)
internal void MakeNonPooledObject(DbConnection owningObject)
{
// Used by DbConnectionFactory to indicate that this object IS NOT part of
// a connection pool.

_connectionPool = null;
_owningObject.Target = owningObject;
_owningObject.SetTarget(owningObject);
_pooledCount = -1;
}

Expand Down Expand Up @@ -368,14 +370,15 @@ internal void PrePush(object expectedOwner)
// ReclaimEmancipatedObjects.

//3 // The following tests are retail assertions of things we can't allow to happen.
if (null == expectedOwner)
bool isAlive = _owningObject.TryGetTarget(out DbConnection connection);
if (expectedOwner == null)
{
if (null != _owningObject.Target)
if (isAlive)
{
throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasOwner); // new unpooled object has an owner
}
}
else if (_owningObject.Target != expectedOwner)
else if (isAlive && connection != expectedOwner)
{
throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasWrongOwner); // unpooled object has incorrect owner
}
Expand All @@ -386,10 +389,10 @@ internal void PrePush(object expectedOwner)

SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionInternal.PrePush|RES|CPOOL> {0}, Preparing to push into pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount);
_pooledCount++;
_owningObject.Target = null; // NOTE: doing this and checking for InternalError.PooledObjectHasOwner degrades the close by 2%
_owningObject.SetTarget(null); // NOTE: doing this and checking for InternalError.PooledObjectHasOwner degrades the close by 2%
}

internal void PostPop(object newOwner)
internal void PostPop(DbConnection newOwner)
{
// Called by DbConnectionPool right after it pulls this from it's pool, we
// take this opportunity to ensure ownership and pool counts are legit.
Expand All @@ -406,12 +409,11 @@ internal void PostPop(object newOwner)
// IMPORTANT NOTE: You must have taken a lock on the object before
// you call this method to prevent race conditions with Clear and
// ReclaimEmancipatedObjects.

if (null != _owningObject.Target)
if (_owningObject.TryGetTarget(out DbConnection _))
{
throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectHasOwner); // pooled connection already has an owner!
}
_owningObject.Target = newOwner;
_owningObject.SetTarget(newOwner);
_pooledCount--;
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionInternal.PostPop|RES|CPOOL> {0}, Preparing to pop from pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount);

Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac
// this is safe since we're about to dispose the
// object and it won't have an owner after that for
// certain.
_owningObject.Target = null;
_owningObject.SetTarget(null);

if (IsTransactionRoot)
{
Expand Down Expand Up @@ -359,7 +359,7 @@ virtual internal void DelegatedTransactionEnded()
}
pool.PutObjectFromTransactedPool(this);
}
else if (-1 == _pooledCount && !_owningObject.IsAlive)
else if (-1 == _pooledCount && !_owningObject.TryGetTarget(out DbConnection _))
{
// When _pooledCount is -1 and the owning object no longer exists,
// it indicates a closed (or leaked), non-pooled connection so
Expand Down
39 changes: 20 additions & 19 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal abstract class DbConnectionInternal
private readonly bool _hidePassword;
private readonly ConnectionState _state;

private readonly WeakReference _owningObject = new WeakReference(null, false); // [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections)
private readonly WeakReference<DbConnection> _owningObject = new WeakReference<DbConnection>(null, false); // [usage must be thread safe] the owning object, when not in the pool. (both Pooled and Non-Pooled connections)

private DbConnectionPool _connectionPool; // the pooler that the connection came from (Pooled connections only)
private DbConnectionPoolCounters _performanceCounters; // the performance counters we're supposed to update
Expand Down Expand Up @@ -80,8 +80,7 @@ internal bool CanBePooled
{
get
{
bool flag = (!_connectionIsDoomed && !_cannotBePooled && !_owningObject.IsAlive);
return flag;
return (!_connectionIsDoomed && !_cannotBePooled && !_owningObject.TryGetTarget(out DbConnection _));
}
}

Expand Down Expand Up @@ -288,8 +287,7 @@ internal bool IsEmancipated
// of the pool and it's owning object is no longer around to
// return it.

bool value = !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.IsAlive;
return value;
return !IsTxRootWaitingForTxEnd && (_pooledCount < 1) && !_owningObject.TryGetTarget(out DbConnection _);
}
}

Expand All @@ -310,13 +308,17 @@ internal int ObjectID
}
}

protected internal object Owner
protected internal DbConnection Owner
{
// We use a weak reference to the owning object so we can identify when
// it has been garbage collected without throwing exceptions.
get
{
return _owningObject.Target;
if (_owningObject.TryGetTarget(out DbConnection connection))
{
return connection;
}
return null;
}
}

Expand Down Expand Up @@ -508,7 +510,7 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac
// this is safe since we're about to dispose the
// object and it won't have an owner after that for
// certain.
_owningObject.Target = null;
_owningObject.SetTarget(null);

if (IsTransactionRoot)
{
Expand Down Expand Up @@ -620,7 +622,7 @@ virtual internal void DelegatedTransactionEnded()
}
pool.PutObjectFromTransactedPool(this);
}
else if (-1 == _pooledCount && !_owningObject.IsAlive)
else if (-1 == _pooledCount && !_owningObject.TryGetTarget(out DbConnection _))
{
// When _pooledCount is -1 and the owning object no longer exists,
// it indicates a closed (or leaked), non-pooled connection so
Expand Down Expand Up @@ -692,14 +694,14 @@ virtual protected internal DataTable GetSchema(DbConnectionFactory factory, DbCo
return metaDataFactory.GetSchema(outerConnection, collectionName, restrictions);
}

internal void MakeNonPooledObject(object owningObject, DbConnectionPoolCounters performanceCounters)
internal void MakeNonPooledObject(DbConnection owningObject, DbConnectionPoolCounters performanceCounters)
{
// Used by DbConnectionFactory to indicate that this object IS NOT part of
// a connection pool.

_connectionPool = null;
_performanceCounters = performanceCounters;
_owningObject.Target = owningObject;
_owningObject.SetTarget(owningObject);
_pooledCount = -1;
}

Expand Down Expand Up @@ -788,14 +790,15 @@ internal void PrePush(object expectedOwner)
// ReclaimEmancipatedObjects.

//3 // The following tests are retail assertions of things we can't allow to happen.
bool isAlive = _owningObject.TryGetTarget(out DbConnection connection);
if (null == expectedOwner)
{
if (null != _owningObject.Target)
if (isAlive)
{
throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasOwner); // new unpooled object has an owner
}
}
else if (_owningObject.Target != expectedOwner)
else if (isAlive && connection != expectedOwner)
{
throw ADP.InternalError(ADP.InternalErrorCode.UnpooledObjectHasWrongOwner); // unpooled object has incorrect owner
}
Expand All @@ -804,14 +807,13 @@ internal void PrePush(object expectedOwner)
throw ADP.InternalError(ADP.InternalErrorCode.PushingObjectSecondTime); // pushing object onto stack a second time
}

//DbConnection x = (expectedOwner as DbConnection);
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionInternal.PrePush|RES|CPOOL> {0}, Preparing to push into pool, owning connection {1}, pooledCount={2}", ObjectID, 0, _pooledCount);

_pooledCount++;
_owningObject.Target = null; // NOTE: doing this and checking for InternalError.PooledObjectHasOwner degrades the close by 2%
_owningObject.SetTarget(null); // NOTE: doing this and checking for InternalError.PooledObjectHasOwner degrades the close by 2%
}

internal void PostPop(object newOwner)
internal void PostPop(DbConnection newOwner)
{
// Called by DbConnectionPool right after it pulls this from it's pool, we
// take this opportunity to ensure ownership and pool counts are legit.
Expand All @@ -828,12 +830,11 @@ internal void PostPop(object newOwner)
// IMPORTANT NOTE: You must have taken a lock on the object before
// you call this method to prevent race conditions with Clear and
// ReclaimEmancipatedObjects.

if (null != _owningObject.Target)
if (_owningObject.TryGetTarget(out DbConnection _))
{
throw ADP.InternalError(ADP.InternalErrorCode.PooledObjectHasOwner); // pooled connection already has an owner!
}
_owningObject.Target = newOwner;
_owningObject.SetTarget(newOwner);
_pooledCount--;

//DbConnection x = (newOwner as DbConnection);
Expand Down