Skip to content

Commit 9d95355

Browse files
committed
Implement return flow and add return/reuse tests.
1 parent 720a71f commit 9d95355

File tree

2 files changed

+207
-26
lines changed

2 files changed

+207
-26
lines changed

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs

Lines changed: 176 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool
4848
public bool IsRunning => State is Running;
4949

5050
/// <inheritdoc />
51-
public TimeSpan LoadBalanceTimeout => throw new NotImplementedException();
51+
public TimeSpan LoadBalanceTimeout => PoolGroupOptions.LoadBalanceTimeout;
5252

5353
/// <inheritdoc />
5454
public DbConnectionPoolGroup PoolGroup => _connectionPoolGroup;
@@ -67,7 +67,7 @@ public DbConnectionPoolState State
6767
}
6868

6969
/// <inheritdoc />
70-
public bool UseLoadBalancing => throw new NotImplementedException();
70+
public bool UseLoadBalancing => PoolGroupOptions.UseLoadBalancing;
7171

7272
private int MaxPoolSize => PoolGroupOptions.MaxPoolSize;
7373
#endregion
@@ -169,7 +169,172 @@ public DbConnectionInternal ReplaceConnection(DbConnection owningObject, DbConne
169169
/// <inheritdoc />
170170
public void ReturnInternalConnection(DbConnectionInternal obj, object owningObject)
171171
{
172-
throw new NotImplementedException();
172+
// Once a connection is closing (which is the state that we're in at
173+
// this point in time) you cannot delegate a transaction to or enlist
174+
// a transaction in it, so we can correctly presume that if there was
175+
// not a delegated or enlisted transaction to start with, that there
176+
// will not be a delegated or enlisted transaction once we leave the
177+
// lock.
178+
179+
lock (obj)
180+
{
181+
// Calling PrePush prevents the object from being reclaimed
182+
// once we leave the lock, because it sets _pooledCount such
183+
// that it won't appear to be out of the pool. What that
184+
// means, is that we're now responsible for this connection:
185+
// it won't get reclaimed if it gets lost.
186+
obj.PrePush(owningObject);
187+
188+
// TODO: Consider using a Cer to ensure that we mark the object for reclaimation in the event something bad happens?
189+
}
190+
191+
if (!CheckConnection(obj))
192+
{
193+
return;
194+
}
195+
196+
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.DeactivateObject|RES|CPOOL> {0}, Connection {1}, Deactivating.", Id, obj.ObjectID);
197+
obj.DeactivateConnection();
198+
199+
bool returnToGeneralPool = false;
200+
bool destroyObject = false;
201+
202+
if (obj.IsConnectionDoomed ||
203+
!obj.CanBePooled ||
204+
State is ShuttingDown)
205+
{
206+
// the object is not fit for reuse -- just dispose of it.
207+
destroyObject = true;
208+
}
209+
else
210+
{
211+
returnToGeneralPool = true;
212+
}
213+
214+
215+
if (returnToGeneralPool)
216+
{
217+
// Only push the connection into the general pool if we didn't
218+
// already push it onto the transacted pool, put it into stasis,
219+
// or want to destroy it.
220+
Debug.Assert(destroyObject == false);
221+
// Statement order is important since we have synchronous completions on the channel.
222+
223+
var written = _idleConnectionWriter.TryWrite(obj);
224+
Debug.Assert(written);
225+
}
226+
else if (destroyObject)
227+
{
228+
// Connections that have been marked as no longer
229+
// poolable (e.g. exceeded their connection lifetime) are not, in fact,
230+
// returned to the general pool
231+
CloseConnection(obj);
232+
}
233+
234+
//-------------------------------------------------------------------------------------
235+
// postcondition
236+
237+
// ensure that the connection was processed
238+
Debug.Assert(returnToGeneralPool == true || destroyObject == true);
239+
}
240+
241+
/// <summary>
242+
/// Checks that the provided connector is live and unexpired and closes it if needed.
243+
/// </summary>
244+
/// <param name="connection"></param>
245+
/// <returns>Returns true if the connector is live and unexpired, otherwise returns false.</returns>
246+
private bool CheckConnection(DbConnectionInternal? connection)
247+
{
248+
// If Clear/ClearAll has been been called since this connector was first opened,
249+
// throw it away. The same if it's broken (in which case CloseConnector is only
250+
// used to update state/perf counter).
251+
//TODO: check clear counter
252+
253+
// An connector could be broken because of a keepalive that occurred while it was
254+
// idling in the pool
255+
// TODO: Consider removing the pool from the keepalive code. The following branch is simply irrelevant
256+
// if keepalive isn't turned on.
257+
if (connection == null)
258+
{
259+
return false;
260+
}
261+
262+
if (!connection.IsConnectionAlive())
263+
{
264+
CloseConnection(connection);
265+
return false;
266+
}
267+
268+
if (LoadBalanceTimeout != TimeSpan.Zero && DateTime.UtcNow > connection.CreateTime + LoadBalanceTimeout)
269+
{
270+
CloseConnection(connection);
271+
return false;
272+
}
273+
274+
return true;
275+
}
276+
277+
/// <summary>
278+
/// Closes the provided connector and adjust pool state accordingly.
279+
/// </summary>
280+
/// <param name="connector">The connector to be closed.</param>
281+
private void CloseConnection(DbConnectionInternal connector)
282+
{
283+
try
284+
{
285+
connector.Dispose();
286+
}
287+
catch
288+
{
289+
//TODO: log error
290+
}
291+
292+
// TODO: check clear counter so that we don't clear new connections
293+
294+
int i;
295+
for (i = 0; i < MaxPoolSize; i++)
296+
{
297+
if (Interlocked.CompareExchange(ref _connections[i], null, connector) == connector)
298+
{
299+
break;
300+
}
301+
}
302+
303+
// If CloseConnection is being called from within OpenNewConnection (e.g. an error happened during a connection initializer which
304+
// causes the connector to Break, and therefore return the connector), then we haven't yet added the connector to Connections.
305+
// In this case, there's no state to revert here (that's all taken care of in OpenNewConnection), skip it.
306+
if (i == MaxPoolSize)
307+
{
308+
return;
309+
}
310+
311+
var numConnections = Interlocked.Decrement(ref _numConnections);
312+
Debug.Assert(numConnections >= 0);
313+
314+
// If a connection has been closed for any reason, we write a null to the idle connection channel to wake up
315+
// a waiter, who will open a new physical connection
316+
// Statement order is important since we have synchronous completions on the channel.
317+
_idleConnectionWriter.TryWrite(null);
318+
}
319+
320+
private void PrepareConnection(DbConnection owningObject, DbConnectionInternal obj)
321+
{
322+
lock (obj)
323+
{ // Protect against Clear and ReclaimEmancipatedObjects, which call IsEmancipated, which is affected by PrePush and PostPop
324+
obj.PostPop(owningObject);
325+
}
326+
try
327+
{
328+
//TODO: pass through transaction
329+
obj.ActivateConnection(null);
330+
}
331+
catch
332+
{
333+
// if Activate throws an exception
334+
// put it back in the pool or have it properly disposed of
335+
this.ReturnInternalConnection(obj, owningObject);
336+
throw;
337+
}
173338
}
174339

175340
/// <inheritdoc />
@@ -239,7 +404,7 @@ private async Task<DbConnectionInternal> GetInternalConnection(DbConnection owni
239404
if (connection != null)
240405
{
241406
// TODO: set connection internal state
242-
// PrepareConnection(owningConnection, connection, transaction);
407+
PrepareConnection(owningConnection, connection);
243408
return connection;
244409
}
245410

@@ -285,10 +450,10 @@ private async Task<DbConnectionInternal> GetInternalConnection(DbConnection owni
285450
}
286451

287452
// TODO: check if connection is still valid
288-
if (connection != null)
453+
if (connection != null && CheckConnection(connection))
289454
{
290455
//TODO: set connection internal state
291-
//PrepareConnection(owningConnection, connection, transaction);
456+
PrepareConnection(owningConnection, connection);
292457
return connection;
293458
}
294459
}
@@ -316,7 +481,7 @@ private async Task<DbConnectionInternal> GetInternalConnection(DbConnection owni
316481
if (connection != null)
317482
{
318483
//TODO: set connection internal state
319-
//PrepareConnection(owningConnection, connection, transaction);
484+
PrepareConnection(owningConnection, connection);
320485
return connection;
321486
}
322487
}
@@ -338,8 +503,10 @@ private async Task<DbConnectionInternal> GetInternalConnection(DbConnection owni
338503
while (_idleConnectionReader.TryRead(out DbConnectionInternal? connection))
339504
{
340505
// TODO: check if connection is still valid
341-
// if (CheckIdleConnection(connection))
342-
return connection;
506+
if (CheckConnection(connection))
507+
{
508+
return connection;
509+
}
343510
}
344511

345512
return null;

0 commit comments

Comments
 (0)