Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool
/// Must be updated using <see cref="Interlocked"/> operations to ensure thread safety.
/// </summary>
private volatile int _isClearing;

/// <summary>
/// Tracks whether <see cref="Shutdown"/> has already initiated the shutdown sequence so that
/// repeated calls are observed as no-ops. Updated atomically via
/// <see cref="Interlocked.CompareExchange(ref int, int, int)"/>.
/// </summary>
private int _shutdownInitiated;
#endregion

/// <summary>
Expand Down Expand Up @@ -254,21 +261,57 @@ public void ReturnInternalConnection(DbConnectionInternal connection, DbConnecti
}
else
{
var written = _idleChannel.TryWrite(connection);
Debug.Assert(written, "Failed to write returning connection to the idle channel.");
if (!_idleChannel.TryWrite(connection))
{
// The channel has been completed (pool is shutting down). Race window
// between the State check above and TryWrite: destroy instead of pooling.
RemoveConnection(connection);
}
Comment thread
priyankatiwari08 marked this conversation as resolved.
}
}

/// <inheritdoc />
public void Shutdown()
{
// No-op for now, warmup will be implemented later.
// idempotent. Compare-and-exchange ensures only one caller performs shutdown work.
if (Interlocked.CompareExchange(ref _shutdownInitiated, 1, 0) != 0)
{
return;
}

SqlClientEventSource.Log.TryPoolerTraceEvent(
"<prov.DbConnectionPool.Shutdown|RES|INFO|CPOOL> {0}", Id);

// FR-001: transition to ShuttingDown. After this point, ReturnInternalConnection
// routes returning connections to RemoveConnection.
State = ShuttingDown;

// FR-002 + FR-007: complete the channel writer so:
// - no further idle connections can be enqueued (TryWrite returns false), and
// - in-flight / future async waiters on ReadAsync fault with ChannelClosedException.
_idleChannel.Complete();

// FR-003: drain remaining buffered idle connections and destroy them. The channel is
// unbounded so all already-enqueued items can be drained synchronously.
while (_idleChannel.TryRead(out DbConnectionInternal? connection))
{
if (connection is not null)
{
RemoveConnection(connection);
}
// null sentinels are wake-up signals only; nothing to destroy.
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we finished the shutdown process here? Should we transition to State Shutdown (which doesn't exixst yet)?

What happens if anything between setting State = ShuttingDown and here throws? Should we allow a subsequent Shutdown() to try again? Should Shutdown() be idempotent?


/// <inheritdoc />
public void Startup()
{
// No-op for now, warmup will be implemented later.
// This pool has no background timers today (idle timeout is enforced lazily in
Comment thread
priyankatiwari08 marked this conversation as resolved.
// IsLiveConnection on retrieval; pruning is not implemented). State is set to Running
// in the constructor, so this is currently the symmetrical counterpart of Shutdown.
// Background work (warmup, pruning timers) will be added here when introduced.
SqlClientEventSource.Log.TryPoolerTraceEvent(
"<prov.DbConnectionPool.Startup|RES|INFO|CPOOL> {0}", Id);
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,19 @@ internal IdleConnectionChannel()
{
var channel = Channel.CreateUnbounded<DbConnectionInternal?>();
_reader = channel.Reader;
//TODO: the channel should be completed on pool shutdown
_writer = channel.Writer;
}

/// <summary>
/// Marks the channel writer as complete. After completion, <see cref="TryWrite"/>
/// returns <see langword="false"/> for any future writes, and any in-flight or future
/// <see cref="ReadAsync"/> waiters will fault with <see cref="System.Threading.Channels.ChannelClosedException"/>
/// once the channel is drained. Used by the connection pool to signal shutdown.
/// </summary>
/// <returns><see langword="true"/> if this call completed the channel; otherwise <see langword="false"/>
/// (channel was already completed).</returns>
internal bool Complete() => _writer.TryComplete();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing tests for this new method.


/// <summary>
/// The number of non-null connections currently in the channel.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,14 @@ public bool IsRunning

private void CleanupCallback(object state)
{
// If the pool is shutting down, skip work. Shutdown disposes the timer, but
// a callback may already be in-flight when Shutdown runs; this guard ensures it does
// not perform pruning or re-arm pool create requests.
if (State == ShuttingDown)
Comment thread
priyankatiwari08 marked this conversation as resolved.
{
return;
}

// Called when the cleanup-timer ticks over.

// This is the automatic pruning method. Every period, we will
Expand Down Expand Up @@ -767,6 +775,13 @@ private void DestroyObject(DbConnectionInternal obj)

private void ErrorCallback(object state)
{
// Skip work if the pool is shutting down. The shutdown path disposes the
// timer; this guard handles the in-flight-callback race.
if (State == ShuttingDown)
{
return;
}

SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.ErrorCallback|RES|CPOOL> {0}, Resetting Error handling.", Id);
_errorOccurred = false;
_waitHandles.ErrorEvent.Reset();
Expand Down Expand Up @@ -956,6 +971,31 @@ private bool TryGetConnection(DbConnection owningObject, uint waitForMultipleObj
{
waitResult = WaitHandle.WaitAny(_waitHandles.GetHandles(allowCreate), unchecked((int)waitForMultipleObjectsTimeout));

// After waking, observe shutdown state and bail out so waiters
// do not spin against a drained pool. If WaitAny consumed a
// PoolSemaphore slot, release it back so the accounting stays
// balanced; otherwise the slot would leak and other waiters
// (or callers that arrive after Shutdown completes its own
// Release loop) would starve.
if (State != Running)
{
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.GetConnection|RES|CPOOL> {0}, Pool is shutting down; abandoning wait.", Id);
Comment thread
priyankatiwari08 marked this conversation as resolved.
if (waitResult == SEMAPHORE_HANDLE || waitResult == WAIT_ABANDONED + SEMAPHORE_HANDLE)
{
Comment on lines +1013 to +1017
try
{
_waitHandles.PoolSemaphore.Release(1);
}
catch (SemaphoreFullException)
{
// Pool semaphore was already saturated by Shutdown's bulk release; safe to ignore.
}
}
Interlocked.Decrement(ref _waitCount);
connection = null;
return false;
}

Comment thread
priyankatiwari08 marked this conversation as resolved.
// From the WaitAny docs: "If more than one object became signaled during
// the call, this is the array index of the signaled object with the
// smallest index value of all the signaled objects." This is important
Expand Down Expand Up @@ -1481,14 +1521,50 @@ public void Startup()
public void Shutdown()
{
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.Shutdown|RES|INFO|CPOOL> {0}", Id);

// Idempotent: subsequent calls observe ShuttingDown and bail.
if (State == ShuttingDown)
{
return;
}
Comment thread
priyankatiwari08 marked this conversation as resolved.
State = ShuttingDown;

// deactivate timer callbacks
Timer t = _cleanupTimer;
_cleanupTimer = null;
if (t != null)
// Dispose all background timers so they no longer schedule new work.
// Note that any timer callback already in flight may still observe State == ShuttingDown
// and short-circuit (see CleanupCallback / ErrorCallback).
Timer cleanup = Interlocked.Exchange(ref _cleanupTimer, null);
Comment thread
priyankatiwari08 marked this conversation as resolved.
cleanup?.Dispose();
Timer error = Interlocked.Exchange(ref _errorTimer, null);
error?.Dispose();

// Wake any threads parked in WaitHandle.WaitAny by releasing as many semaphore
// slots as there are recorded waiters. Using _waitCount (rather than MaxPoolSize)
// avoids ArgumentOutOfRangeException when MaxPoolSize == 0 (unlimited) and ensures
// we wake every parked waiter even when _waitCount exceeds MaxPoolSize. Waiters
// observe State != Running after wake-up and bail.
int waitersToWake = Volatile.Read(ref _waitCount);
if (waitersToWake > 0)
{
try
{
_waitHandles.PoolSemaphore.Release(waitersToWake);
}
catch (SemaphoreFullException)
{
// Semaphore already saturated; nothing to do.
}
}

// Drain idle stacks and destroy contained connections. Active checked-out
// connections are destroyed when they are returned (see DeactivateObject's
// State is ShuttingDown branch).
while (_stackNew.TryPop(out DbConnectionInternal newObj))
Comment thread
priyankatiwari08 marked this conversation as resolved.
Outdated
{
DestroyObject(newObj);
}
while (_stackOld.TryPop(out DbConnectionInternal oldObj))
{
t.Dispose();
DestroyObject(oldObj);
}
}

Expand Down
Loading
Loading