Skip to content

Commit 5585b10

Browse files
committed
Address ben's comments.
1 parent cbf8ba9 commit 5585b10

File tree

1 file changed

+27
-20
lines changed

1 file changed

+27
-20
lines changed

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -307,16 +307,16 @@ out DbConnectionInternal? internalConnection
307307
Assert.NotNull(internalConnection);
308308
}
309309

310-
// Use multiple ManualResetEventSlim to ensure proper ordering
311-
using ManualResetEventSlim firstTaskReady = new(false);
312-
using ManualResetEventSlim secondTaskReady = new(false);
313-
using ManualResetEventSlim startRequests = new(false);
310+
// Use TaskCompletionSource for coordination to avoid mixing async/await with native synchronization
311+
TaskCompletionSource<bool> firstTaskReady = new();
312+
TaskCompletionSource<bool> secondTaskReady = new();
313+
TaskCompletionSource<bool> startRequests = new();
314314

315315
// Act
316-
var recycledTask = Task.Run(() =>
316+
var recycledTask = Task.Run(async () =>
317317
{
318-
firstTaskReady.Set();
319-
startRequests.Wait();
318+
firstTaskReady.SetResult(true);
319+
await startRequests.Task;
320320
pool.TryGetConnection(
321321
new SqlConnection("Timeout=5000"),
322322
null,
@@ -326,12 +326,14 @@ out DbConnectionInternal? recycledConnection
326326
return recycledConnection;
327327
});
328328

329-
var failedTask = Task.Run(() =>
329+
var failedTask = Task.Run(async () =>
330330
{
331-
secondTaskReady.Set();
332-
startRequests.Wait();
333-
// Add a small delay to ensure this request comes after the first
334-
Thread.Sleep(50);
331+
secondTaskReady.SetResult(true);
332+
await startRequests.Task;
333+
// Add a small delay to ensure this request comes after the first.
334+
// This is necessary because the channel-based pool queues requests in FIFO order,
335+
// and we need to guarantee the order for this test to be deterministic.
336+
await Task.Delay(50);
335337
pool.TryGetConnection(
336338
new SqlConnection("Timeout=1"),
337339
null,
@@ -342,16 +344,18 @@ out DbConnectionInternal? failedConnection
342344
});
343345

344346
// Wait for both tasks to be ready before starting the requests
345-
firstTaskReady.Wait();
346-
secondTaskReady.Wait();
347+
await firstTaskReady.Task;
348+
await secondTaskReady.Task;
347349

348-
// Use SpinWait to ensure both tasks are actually waiting
349-
SpinWait.SpinUntil(() => false, 100);
350+
// Allow both tasks to reach their wait state before proceeding
351+
await Task.Delay(100);
350352

351353
// Start both requests
352-
startRequests.Set();
354+
startRequests.SetResult(true);
353355

354-
// Give time for both requests to be queued
356+
// Give time for both requests to be queued.
357+
// This delay ensures that both TryGetConnection calls have been made and are waiting in the channel
358+
// before we return the connection, which is necessary to test FIFO ordering.
355359
await Task.Delay(200);
356360

357361
// Return the connection which should satisfy the first queued request
@@ -401,7 +405,9 @@ out DbConnectionInternal? internalConnection
401405
out DbConnectionInternal? recycledConnection
402406
);
403407

404-
// Ensure sufficient time for the recycled connection request to be fully queued
408+
// Ensure sufficient time for the recycled connection request to be fully queued.
409+
// This delay is necessary because the channel-based pool queues async requests,
410+
// and we need to guarantee the first request is in the queue before the second one.
405411
await Task.Delay(200);
406412

407413
var exceeded2 = pool.TryGetConnection(
@@ -411,7 +417,8 @@ out DbConnectionInternal? recycledConnection
411417
out DbConnectionInternal? failedConnection
412418
);
413419

414-
// Ensure the second request is also queued
420+
// Ensure the second request is also queued before returning the connection.
421+
// This guarantees that both requests are waiting in FIFO order.
415422
await Task.Delay(100);
416423

417424
pool.ReturnInternalConnection(firstConnection!, firstOwningConnection);

0 commit comments

Comments
 (0)