Skip to content

Commit 12934f4

Browse files
cheenamalhotracheenamalhotra
authored
Fix connection pool concurrency issue (#3632)
1 parent b2fc7e0 commit 12934f4

File tree

4 files changed

+563
-29
lines changed

4 files changed

+563
-29
lines changed

‎src/Microsoft.Data.SqlClient.sln‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,18 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Data.SqlClient.Un
303303
EndProject
304304
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Common", "Microsoft.Data.SqlClient\tests\Common\Common.csproj", "{67128EC0-30F5-6A98-448B-55F88A1DE707}"
305305
EndProject
306+
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Stress", "Stress", "{02EA681E-C7D8-13C7-8484-4AC65E1B71E8}"
307+
EndProject
308+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "IMonitorLoader", "Microsoft.Data.SqlClient\tests\StressTests\IMonitorLoader\IMonitorLoader.csproj", "{1A29B520-D16A-35F2-5CAC-64573C86E63D}"
309+
EndProject
310+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SqlClient.Stress.Common", "Microsoft.Data.SqlClient\tests\StressTests\SqlClient.Stress.Common\SqlClient.Stress.Common.csproj", "{2AA12D54-540B-E515-CB82-80D691C9DCF1}"
311+
EndProject
312+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SqlClient.Stress.Framework", "Microsoft.Data.SqlClient\tests\StressTests\SqlClient.Stress.Framework\SqlClient.Stress.Framework.csproj", "{92D9C6D6-6925-1AD1-69FA-485F83943BD2}"
313+
EndProject
314+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SqlClient.Stress.Runner", "Microsoft.Data.SqlClient\tests\StressTests\SqlClient.Stress.Runner\SqlClient.Stress.Runner.csproj", "{4A9C11F4-9577-ABEC-C070-83A194746D9B}"
315+
EndProject
316+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SqlClient.Stress.Tests", "Microsoft.Data.SqlClient\tests\StressTests\SqlClient.Stress.Tests\SqlClient.Stress.Tests.csproj", "{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}"
317+
EndProject
306318
Global
307319
GlobalSection(SolutionConfigurationPlatforms) = preSolution
308320
Debug|Any CPU = Debug|Any CPU
@@ -571,6 +583,66 @@ Global
571583
{67128EC0-30F5-6A98-448B-55F88A1DE707}.Release|x64.Build.0 = Release|x64
572584
{67128EC0-30F5-6A98-448B-55F88A1DE707}.Release|x86.ActiveCfg = Release|x86
573585
{67128EC0-30F5-6A98-448B-55F88A1DE707}.Release|x86.Build.0 = Release|x86
586+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
587+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Debug|Any CPU.Build.0 = Debug|Any CPU
588+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Debug|x64.ActiveCfg = Debug|Any CPU
589+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Debug|x64.Build.0 = Debug|Any CPU
590+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Debug|x86.ActiveCfg = Debug|Any CPU
591+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Debug|x86.Build.0 = Debug|Any CPU
592+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Release|Any CPU.ActiveCfg = Release|Any CPU
593+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Release|Any CPU.Build.0 = Release|Any CPU
594+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Release|x64.ActiveCfg = Release|Any CPU
595+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Release|x64.Build.0 = Release|Any CPU
596+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Release|x86.ActiveCfg = Release|Any CPU
597+
{1A29B520-D16A-35F2-5CAC-64573C86E63D}.Release|x86.Build.0 = Release|Any CPU
598+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
599+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Debug|Any CPU.Build.0 = Debug|Any CPU
600+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Debug|x64.ActiveCfg = Debug|Any CPU
601+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Debug|x64.Build.0 = Debug|Any CPU
602+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Debug|x86.ActiveCfg = Debug|Any CPU
603+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Debug|x86.Build.0 = Debug|Any CPU
604+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Release|Any CPU.ActiveCfg = Release|Any CPU
605+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Release|Any CPU.Build.0 = Release|Any CPU
606+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Release|x64.ActiveCfg = Release|Any CPU
607+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Release|x64.Build.0 = Release|Any CPU
608+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Release|x86.ActiveCfg = Release|Any CPU
609+
{2AA12D54-540B-E515-CB82-80D691C9DCF1}.Release|x86.Build.0 = Release|Any CPU
610+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
611+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Debug|Any CPU.Build.0 = Debug|Any CPU
612+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Debug|x64.ActiveCfg = Debug|Any CPU
613+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Debug|x64.Build.0 = Debug|Any CPU
614+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Debug|x86.ActiveCfg = Debug|Any CPU
615+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Debug|x86.Build.0 = Debug|Any CPU
616+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Release|Any CPU.ActiveCfg = Release|Any CPU
617+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Release|Any CPU.Build.0 = Release|Any CPU
618+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Release|x64.ActiveCfg = Release|Any CPU
619+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Release|x64.Build.0 = Release|Any CPU
620+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Release|x86.ActiveCfg = Release|Any CPU
621+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2}.Release|x86.Build.0 = Release|Any CPU
622+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
623+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Debug|Any CPU.Build.0 = Debug|Any CPU
624+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Debug|x64.ActiveCfg = Debug|Any CPU
625+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Debug|x64.Build.0 = Debug|Any CPU
626+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Debug|x86.ActiveCfg = Debug|Any CPU
627+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Debug|x86.Build.0 = Debug|Any CPU
628+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Release|Any CPU.ActiveCfg = Release|Any CPU
629+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Release|Any CPU.Build.0 = Release|Any CPU
630+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Release|x64.ActiveCfg = Release|Any CPU
631+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Release|x64.Build.0 = Release|Any CPU
632+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Release|x86.ActiveCfg = Release|Any CPU
633+
{4A9C11F4-9577-ABEC-C070-83A194746D9B}.Release|x86.Build.0 = Release|Any CPU
634+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
635+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Debug|Any CPU.Build.0 = Debug|Any CPU
636+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Debug|x64.ActiveCfg = Debug|Any CPU
637+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Debug|x64.Build.0 = Debug|Any CPU
638+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Debug|x86.ActiveCfg = Debug|Any CPU
639+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Debug|x86.Build.0 = Debug|Any CPU
640+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Release|Any CPU.ActiveCfg = Release|Any CPU
641+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Release|Any CPU.Build.0 = Release|Any CPU
642+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Release|x64.ActiveCfg = Release|Any CPU
643+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Release|x64.Build.0 = Release|Any CPU
644+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Release|x86.ActiveCfg = Release|Any CPU
645+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C}.Release|x86.Build.0 = Release|Any CPU
574646
EndGlobalSection
575647
GlobalSection(SolutionProperties) = preSolution
576648
HideSolutionNode = FALSE
@@ -621,6 +693,12 @@ Global
621693
{AD738BD4-6A02-4B88-8F93-FBBBA49A74C8} = {4CAE9195-4F1A-4D48-854C-1C9FBC512C66}
622694
{4461063D-2F2B-274C-7E6F-F235119D258E} = {0CC4817A-12F3-4357-912C-09315FAAD008}
623695
{67128EC0-30F5-6A98-448B-55F88A1DE707} = {0CC4817A-12F3-4357-912C-09315FAAD008}
696+
{02EA681E-C7D8-13C7-8484-4AC65E1B71E8} = {0CC4817A-12F3-4357-912C-09315FAAD008}
697+
{1A29B520-D16A-35F2-5CAC-64573C86E63D} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
698+
{2AA12D54-540B-E515-CB82-80D691C9DCF1} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
699+
{92D9C6D6-6925-1AD1-69FA-485F83943BD2} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
700+
{4A9C11F4-9577-ABEC-C070-83A194746D9B} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
701+
{FAA1E517-581A-D3DC-BAC9-FAD1D5A5142C} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
624702
EndGlobalSection
625703
GlobalSection(ExtensibilityGlobals) = postSolution
626704
SolutionGuid = {01D48116-37A2-4D33-B9EC-94793C702431}

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

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,41 @@ internal WaitHandle[] GetHandles(bool withCreate)
383383
}
384384
}
385385

386+
/// <summary>
387+
/// Helper class to obtain and release a semaphore.
388+
/// </summary>
389+
internal class SemaphoreHolder : IDisposable
390+
{
391+
private readonly Semaphore _semaphore;
392+
393+
/// <summary>
394+
/// Whether the semaphore was successfully obtained within the timeout.
395+
/// </summary>
396+
internal bool Obtained { get; private set; }
397+
398+
/// <summary>
399+
/// Obtains the semaphore, waiting up to the specified timeout.
400+
/// </summary>
401+
/// <param name="semaphore"></param>
402+
/// <param name="timeout"></param>
403+
internal SemaphoreHolder(Semaphore semaphore, int timeout)
404+
{
405+
_semaphore = semaphore;
406+
Obtained = _semaphore.WaitOne(timeout);
407+
}
408+
409+
/// <summary>
410+
/// Releases the semaphore if it was successfully obtained.
411+
/// </summary>
412+
public void Dispose()
413+
{
414+
if (Obtained)
415+
{
416+
_semaphore.Release(1);
417+
}
418+
}
419+
}
420+
386421
private const int MAX_Q_SIZE = 0x00100000;
387422

388423
// The order of these is important; we want the WaitAny call to be signaled
@@ -1281,17 +1316,11 @@ private bool TryGetConnection(DbConnection owningObject, uint waitForMultipleObj
12811316

12821317
if (onlyOneCheckConnection)
12831318
{
1284-
if (_waitHandles.CreationSemaphore.WaitOne(unchecked((int)waitForMultipleObjectsTimeout)))
1319+
using SemaphoreHolder semaphoreHolder = new(_waitHandles.CreationSemaphore, unchecked((int)waitForMultipleObjectsTimeout));
1320+
if (semaphoreHolder.Obtained)
12851321
{
1286-
try
1287-
{
1288-
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.GetConnection|RES|CPOOL> {0}, Creating new connection.", Id);
1289-
obj = UserCreateRequest(owningObject, userOptions);
1290-
}
1291-
finally
1292-
{
1293-
_waitHandles.CreationSemaphore.Release(1);
1294-
}
1322+
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.GetConnection|RES|CPOOL> {0}, Creating new connection.", Id);
1323+
obj = UserCreateRequest(owningObject, userOptions);
12951324
}
12961325
else
12971326
{
@@ -1510,14 +1539,13 @@ private void PoolCreateRequest(object state)
15101539
{
15111540
return;
15121541
}
1513-
int waitResult = BOGUS_HANDLE;
1514-
1542+
15151543
try
15161544
{
15171545
// Obtain creation mutex so we're the only one creating objects
1518-
// and we must have the wait result
1519-
waitResult = WaitHandle.WaitAny(_waitHandles.GetHandles(withCreate: true), CreationTimeout);
1520-
if (CREATION_HANDLE == waitResult)
1546+
using SemaphoreHolder semaphoreHolder = new(_waitHandles.CreationSemaphore, CreationTimeout);
1547+
1548+
if (semaphoreHolder.Obtained)
15211549
{
15221550
DbConnectionInternal newObj;
15231551

@@ -1552,17 +1580,12 @@ private void PoolCreateRequest(object state)
15521580
}
15531581
}
15541582
}
1555-
else if (WaitHandle.WaitTimeout == waitResult)
1583+
else
15561584
{
15571585
// do not wait forever and potential block this worker thread
15581586
// instead wait for a period of time and just requeue to try again
15591587
QueuePoolCreateRequest();
15601588
}
1561-
else
1562-
{
1563-
// trace waitResult and ignore the failure
1564-
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.PoolCreateRequest|RES|CPOOL> {0}, PoolCreateRequest called WaitForSingleObject failed {1}", Id, waitResult);
1565-
}
15661589
}
15671590
catch (Exception e)
15681591
{
@@ -1576,14 +1599,6 @@ private void PoolCreateRequest(object state)
15761599
// thrown to the user the next time they request a connection.
15771600
SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.PoolCreateRequest|RES|CPOOL> {0}, PoolCreateRequest called CreateConnection which threw an exception: {1}", Id, e);
15781601
}
1579-
finally
1580-
{
1581-
if (CREATION_HANDLE == waitResult)
1582-
{
1583-
// reuse waitResult and ignore its value
1584-
_waitHandles.CreationSemaphore.Release(1);
1585-
}
1586-
}
15871602
}
15881603
}
15891604
}

‎src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@
259259
<None Include="SQL\ParameterTest\SqlParameterTest_ReleaseMode_Azure.bsl" />
260260
</ItemGroup>
261261
<ItemGroup Condition="'$(TestSet)' == '' OR '$(TestSet)' == '3'">
262+
<Compile Include="SQL\ConnectionPoolTest\ConnectionPoolStressTest.cs" />
262263
<Compile Include="TracingTests\MetricsTest.cs" />
263264
<Compile Include="TracingTests\DiagnosticTest.cs" />
264265
<Compile Include="TracingTests\FakeDiagnosticListenerObserver.cs" />

0 commit comments

Comments
 (0)