8000 Fix internal connection activation, deactivation, and internal state … · dotnet/SqlClient@30b13c7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 30b13c7

Browse files
mdaiglemdaigle
committed
Fix internal connection activation, deactivation, and internal state setting.
1 parent 59da4d6 commit 30b13c7

File tree

1 file changed

+65
-29
lines changed
  • src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool

1 file changed

+65
-29
lines changed

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/BetterSyncPool.cs

Lines changed: 65 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#if NET
66
using System;
77
using System.Collections.Concurrent;
8+
using System.Collections.Generic;
89
using System.Data;
910
using System.Data.Common;
1011
using System.Diagnostics;
@@ -96,19 +97,44 @@ internal override bool TryGetConnection(DbConnection owningObject, TaskCompletio
9697
{
9798
ThreadPool.QueueUserWorkItem(async (_) =>
9899
{
99-
var connection = await GetInternalConnection(owningObject, userOptions, TimeSpan.Zero, false, CancellationToken.None).ConfigureAwait(false);
100+
//TODO: use same timespan everywhere and tick down for queueuing and actual connection opening work
101+
var connection = await GetInternalConnection(owningObject, userOptions, TimeSpan.FromSeconds(owningObject.ConnectionTimeout), false, CancellationToken.None).ConfigureAwait(false);
102+
//TODO set transaction if necessary
103+
PrepareConnection(owningObject, connection, null);
100104
taskCompletionSource.SetResult(connection);
101105
});
102106
connection = null;
103107
return false;
104108
}
105109
else
106110
{
107-
connection = GetInternalConnection(owningObject, userOptions, TimeSpan.Zero, false, CancellationToken.None).Result;
111+
//TODO: use same timespan everywhere and tick down for queueuing and actual connection opening work
112+
connection = GetInternalConnection(owningObject, userOptions, TimeSpan.FromSeconds(owningObject.ConnectionTimeout), false, CancellationToken.None).Result;
113+
//TODO set transaction if necessary
114+
PrepareConnection(owningObject, connection, null);
108115
return connection is not null;
109116
}
110117
}
111118

119+
private void PrepareConnection(DbConnection owningObject, DbConnectionInternal obj, Transaction? transaction)
120+
{
121+
lock (obj)
122+
{ // Protect against Clear and ReclaimEmancipatedObjects, which call IsEmancipated, which is affected by PrePush and PostPop
123+
obj.PostPop(owningObject);
124+
}
125+
try
126+
{
127+
obj.ActivateConnection(transaction);
128+
}
129+
catch
130+
{
131+
// if Activate throws an exception
132+
// put it back in the pool or have it properly disposed of
133+
this.ReturnInternalConnection(obj, owningObject);
134+
throw;
135+
}
136+
}
137+
112138
/// <summary>
113139
/// Creates a new connection to replace an existing connection
114140
/// </summary>
@@ -125,9 +151,40 @@ internal override DbConnectionInternal ReplaceConnection(DbConnection owningObje
125151

126152
}
127153

128-
internal override void ReturnInternalConnection(DbConnectionInternal obj, object owningObject)
154+
internal override void ReturnInternalConnection(DbConnectionInternal connector, object? owningObject)
129155
{
130-
ReturnInternalConnection(obj);
156+
// Once a connection is closing (which is the state that we're in at
157+
// this point in time) you cannot delegate a transaction to or enlist
158+
// a transaction in it, so we can correctly presume that if there was
159+
// not a delegated or enlisted transaction to start with, that there
160+
// will not be a delegated or enlisted transaction once we leave the
161+
// lock.
162+
163+
lock (connector)
164+
{
165+
// Calling PrePush prevents the object from being reclaimed
166+
// once we leave the lock, because it sets _pooledCount such
167+
// that it won't appear to be out of the pool. What that
168+
// means, is that we're now responsible for this connection:
169+
// it won't get reclaimed if it gets lost.
170+
connector.PrePush(owningObject);
171+
172+
// TODO: Consider using a Cer to ensure that we mark the object for reclaimation in the event something bad happens?
173+
}
174+
175+
//TODO: verify transaction state
176+
177+
if (!CheckConnector(connector))
178+
{
179+
return;
180+
}
181+
182+
connector.DeactivateConnection();
183+
184+
// Statement order is important since we have synchronous completions on the channel.
185+
Interlocked.Increment(ref _idleCount);
186+
var written = _idleConnectorWriter.TryWrite(connector);
187+
Debug.Assert(written);
131188
}
132189

133190
internal override void PutObjectFromTransactedPool(DbConnectionInternal obj)
@@ -306,7 +363,6 @@ internal async ValueTask<DbConnectionInternal> GetInternalConnection(DbConnectio
306363
if (connector != null)
307364
{
308365
// TODO: transactions
309-
connector.ActivateConnection(null);
310366
return connector;
311367
}
312368

@@ -315,7 +371,6 @@ internal async ValueTask<DbConnectionInternal> GetInternalConnection(DbConnectio
315371
if (connector != null)
316372
{
317373
// TODO: transactions
318-
connector.ActivateConnection(null);
319374
return connector;
320375
}
321376

@@ -360,7 +415,6 @@ internal async ValueTask<DbConnectionInternal> GetInternalConnection(DbConnectio
360415
if (CheckIdleConnector(connector))
361416
{
362417
// TODO: transactions
363-
connector.ActivateConnection(null);
364418
return connector;
365419
}
366420
}
@@ -369,8 +423,7 @@ internal async ValueTask<DbConnectionInternal> GetInternalConnection(DbConnectio
369423
cancellationToken.ThrowIfCancellationRequested();
370424
Debug.Assert(finalToken.IsCancellationRequested);
371425

372-
//TODO: exceptions from resource file
373-
throw new Exception("Pool exhausted", new TimeoutException());
426+
throw ADP.PooledOpenTimeout();
374427
}
375428
catch (ChannelClosedException)
376429
{
@@ -588,6 +641,8 @@ internal readonly struct OpenInternalConnectionState
588641
throw ADP.InternalError(ADP.InternalErrorCode.NewObjectCannotBePooled); // CreateObject succeeded, but non-poolable object
589642
}
590643

644+
newConnection.PrePush(null);
645+
591646
int i;
592647
for (i = 0; i < state.Pool.MaxPoolSize; i++)
593648
{
@@ -624,25 +679,6 @@ internal readonly struct OpenInternalConnectionState
624679
}
625680
}
626681

627-
/// <inheritdoc/>
628-
internal void ReturnInternalConnection(DbConnectionInternal connector)
629-
{
630-
631-
//TODO: verify transaction state
632-
633-
if (!CheckConnector(connector))
634-
{
635-
return;
636-
}
637-
638-
connector.DeactivateConnection();
639-
640-
// Statement order is important since we have synchronous completions on the channel.
641-
Interlocked.Increment(ref _idleCount);
642-
var written = _idleConnectorWriter.TryWrite(connector);
643-
Debug.Assert(written);
644-
}
645-
646682
/// <summary>
647683
/// Initiates a task to wait on the pruning timer. Spins off child tasks to perform pruning
648684
/// each time the timer ticks.
@@ -846,7 +882,7 @@ async Task _WarmUp(CancellationToken ct)
846882

847883
// The connector has never been used, so it's safe to immediately return it to the
848884
// pool without resetting it.
849-
ReturnInternalConnection(connector);
885+
ReturnInternalConnection(connector, null);
850886
}
851887
}
852888
}

0 commit comments

Comments
 (0)
0