diff --git a/.github/instructions/features.instructions.md b/.github/instructions/features.instructions.md index 34262b8db6..f7f5fe607a 100644 --- a/.github/instructions/features.instructions.md +++ b/.github/instructions/features.instructions.md @@ -254,7 +254,7 @@ AppContext switches allow runtime behavior changes without modifying connection | `Switch.Microsoft.Data.SqlClient.TruncateScaledDecimal` | `false` | Truncates scaled decimal values instead of rounding | | `Switch.Microsoft.Data.SqlClient.UseCompatibilityAsyncBehaviour` | `false` | Uses legacy async behavior for compatibility | | `Switch.Microsoft.Data.SqlClient.UseCompatibilityProcessSni` | `false` | Uses legacy SNI processing path | -| `Switch.Microsoft.Data.SqlClient.UseConnectionPoolV2` | `false` | Enables the new `ChannelDbConnectionPool` implementation | +| `Switch.Microsoft.Data.SqlClient.UseConnectionPoolV2` | `true` | Enables the new `ChannelDbConnectionPool` implementation; set to `false` to restore the legacy `WaitHandleDbConnectionPool` | | `Switch.Microsoft.Data.SqlClient.UseManagedNetworkingOnWindows` | `false` | Forces managed SNI on Windows (instead of native SNI) | | `Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin` | `false` | Sets 1-second minimum in login timeout calculations | diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index a547ce0fd4..0cedf611e3 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -424,6 +424,21 @@ public DbConnectionInternal ReplaceConnection( SqlClientEventSource.Log.TryPoolerTraceEvent( "ChannelDbConnectionPool.ReplaceConnection | INFO | {0}, replacing connection.", Id); + // A broken connection can be returned and removed from the pool before connection + // resiliency asks for its replacement. Its slot is already free, so acquire the + // replacement through the normal path instead of trying to swap a missing slot. + if (!ReferenceEquals(oldConnection.Pool, this)) + { + return GetInternalConnection( + owningObject, + async: false, + timeout, + oldConnection.EnlistedTransaction) + .ConfigureAwait(false) + .GetAwaiter() + .GetResult(); + } + // First, prefer to get an idle connection from the pool. // If one is available, we can avoid the cost of creating a new connection. DbConnectionInternal? newConnection = GetIdleConnection(); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index 06bf6c4f0e..269b74c97c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -580,12 +580,12 @@ public static bool UseCompatibilityAsyncBehaviour /// pool implementation. When set to false, the connection pool will use /// the legacy V1 implementation. /// - /// The default value of this switch is false. + /// The default value of this switch is true. /// public static bool UseConnectionPoolV2 => AcquireAndReturn( UseConnectionPoolV2String, - defaultValue: false, + defaultValue: true, ref s_useConnectionPoolV2); /// diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/AmbientTransactionFailureTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/AmbientTransactionFailureTest.cs deleted file mode 100644 index b5069f6197..0000000000 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/AmbientTransactionFailureTest.cs +++ /dev/null @@ -1,93 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Runtime.InteropServices; -using System.Threading; -using System.Threading.Tasks; -using System.Transactions; -using Xunit; - -namespace Microsoft.Data.SqlClient.Tests -{ - public class AmbientTransactionFailureTest - { - private static readonly bool s_isNotArmProcess = TestUtility.IsNotArmProcess; - private static readonly string s_servername = Guid.NewGuid().ToString(); - private static readonly string s_connectionStringWithEnlistAsDefault = $"Data Source={s_servername}; Integrated Security=true; Connect Timeout=1;"; - private static readonly string s_connectionStringWithEnlistOff = $"Data Source={s_servername}; Integrated Security=true; Connect Timeout=1;Enlist=False"; - - private static Action ConnectToServer = (connectionString) => - { - using (SqlConnection connection = new SqlConnection(connectionString)) - { - connection.Open(); - } - }; - - private static Action ConnectToServerTask = (connectionString) => - { - using (SqlConnection connection = new SqlConnection(connectionString)) - { - connection.OpenAsync(); - } - }; - - private static Func ConnectToServerInTransactionScopeTask = (connectionString) => - { - return Task.Run(() => - { - using (TransactionScope scope = new TransactionScope()) - { - ConnectToServerTask(connectionString); - } - }); - }; - - private static Action ConnectToServerInTransactionScope = (connectionString) => - { - using (TransactionScope scope = new TransactionScope()) - { - ConnectToServer(connectionString); - } - }; - - private static Action EnlistConnectionInTransaction = (connectionString) => - { - using (TransactionScope scope = new TransactionScope()) - { - SqlConnection connection = new SqlConnection(connectionString); - connection.EnlistTransaction(Transaction.Current); - } - }; - - public static readonly object[][] ExceptionTestDataForSqlException = - { - new object[] { ConnectToServerInTransactionScope, s_connectionStringWithEnlistOff }, - new object[] { ConnectToServer, s_connectionStringWithEnlistAsDefault } - }; - - public static readonly object[][] ExceptionTestDataForNotSupportedException = - { - new object[] { ConnectToServerInTransactionScope, s_connectionStringWithEnlistAsDefault }, - new object[] { EnlistConnectionInTransaction, s_connectionStringWithEnlistAsDefault }, - new object[] { EnlistConnectionInTransaction, s_connectionStringWithEnlistOff } - }; - - [ConditionalTheory(nameof(s_isNotArmProcess))] // https://github.com/dotnet/corefx/issues/21598 - [MemberData( - nameof(ExceptionTestDataForSqlException), - // xUnit can't consistently serialize the data for this test, so we - // disable enumeration of the test data to avoid warnings on the - // console. - DisableDiscoveryEnumeration = true)] - public void TestSqlException(Action connectAction, string connectionString) - { - Assert.Throws(() => - { - connectAction(connectionString); - }); - } - } -} diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs index a8baee786c..6d65189abf 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs @@ -65,15 +65,26 @@ public async Task TestPacketNumberWraparound() Stopwatch stopwatch = new(); stopwatch.Start(); + // Task.Factory.StartNew with an async delegate returns a Task, so it must be + // unwrapped before use in Task.WhenAny below. Without Unwrap(), WhenAny would observe + // only the outer task (which completes as soon as the async lambda hits its first + // await) instead of the actual completion of RunPacketNumberWraparound. Task actionTask = Task.Factory.StartNew( - async () => await RunPacketNumberWraparound(enumerator, cancellationTokenSource.Token), - TaskCreationOptions.DenyChildAttach | TaskCreationOptions.LongRunning); + () => RunPacketNumberWraparound(enumerator, cancellationTokenSource.Token), + TaskCreationOptions.DenyChildAttach | TaskCreationOptions.LongRunning).Unwrap(); Task timeoutTask = Task.Delay(TimeSpan.FromSeconds(60), cancellationTokenSource.Token); - await Task.WhenAny(actionTask, timeoutTask); + Task completedTask = await Task.WhenAny(actionTask, timeoutTask); stopwatch.Stop(); cancellationTokenSource.Cancel(); + // Propagate any unexpected failure from the action task (e.g. a connection open + // failure) instead of letting it surface only as a low enumerator count below. + if (completedTask == actionTask) + { + await actionTask; + } + // Assert Assert.True( enumerator.MaxCount == enumerator.Count, diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolReplaceConnectionTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolReplaceConnectionTest.cs index 6df01bc1d9..2538cf555a 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolReplaceConnectionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolReplaceConnectionTest.cs @@ -543,6 +543,47 @@ public void ReplaceConnection_NoIdleConnection_CreatesNew() Assert.Equal(1, pool.Count); } + /// + /// Verifies connection resiliency can replace a broken connection after its return path has + /// already removed it from the pool and released its slot. + /// + [Fact] + public void ReplaceConnection_OldConnectionAlreadyRemoved_AcquiresNewSlot() + { + // Arrange + var poolGroupOptions = new DbConnectionPoolGroupOptions( + poolByIdentity: false, + minPoolSize: 0, + maxPoolSize: 1, + creationTimeout: 15, + loadBalanceTimeout: 0, + hasTransactionAffinity: true, + idleTimeout: 0 + ); + var pool = ConstructPool(_factory, poolGroupOptions); + SqlConnection owner = new(); + + pool.TryGetConnection(owner, null, TimeoutTimer.StartNew(TimeSpan.FromSeconds(15)), out DbConnectionInternal? connection); + StubDbConnectionInternal oldConnection = Assert.IsType(connection); + oldConnection.Doom(); + pool.ReturnInternalConnection(oldConnection, owner); + + Assert.Equal(0, pool.Count); + Assert.Null(oldConnection.Pool); + + // Act + DbConnectionInternal newConnection = pool.ReplaceConnection( + owner, + oldConnection, + TimeoutTimer.StartNew(TimeSpan.FromSeconds(15))); + + // Assert + Assert.NotSame(oldConnection, newConnection); + Assert.Equal(1, pool.Count); + Assert.Same(pool, newConnection.Pool); + Assert.Same(owner, newConnection.Owner); + } + #endregion #region Blocking Period @@ -729,6 +770,8 @@ internal override void ResetConnection() { return; } + + internal void Doom() => DoomThisConnection(); } #endregion diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index d647ab0914..9f53a8b8be 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -588,6 +588,29 @@ out DbConnectionInternal? internalConnection Assert.Equal(ADP.PooledOpenTimeout().Message, ex.Message); } + /// + /// Verifies that an expired caller timeout prevents physical connection creation. + /// + [Fact] + public void GetConnectionExpiredTimeout_DoesNotAttemptPhysicalConnection() + { + // Arrange + var connectionFactory = new CountingTimeoutConnectionFactory(); + var pool = ConstructPool(connectionFactory); + + // Act + InvalidOperationException exception = Assert.Throws(() => + pool.TryGetConnection( + new SqlConnection(), + taskCompletionSource: null, + TimeoutTimer.StartExpired(), + out _)); + + // Assert + Assert.Equal(ADP.PooledOpenTimeout().Message, exception.Message); + Assert.Equal(0, connectionFactory.CreateCount); + } + /// /// Verifies under concurrent synchronous load that the pool never grows beyond its /// configured maximum size and continues to serve requests safely. diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs index ff70c17f4b..f0ff098bd1 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs @@ -63,7 +63,7 @@ public void TestDefaultAppContextSwitchValues() Assert.True(switchesHelper.UseCompatibilityProcessSni); Assert.True(switchesHelper.UseCompatibilityAsyncBehaviour); Assert.True(switchesHelper.UseLegacyIdleTimeoutBehavior); - Assert.False(switchesHelper.UseConnectionPoolV2); + Assert.True(switchesHelper.UseConnectionPoolV2); Assert.False(switchesHelper.UseOverallConnectTimeoutForPoolWait); Assert.False(switchesHelper.TruncateScaledDecimal); Assert.False(switchesHelper.IgnoreServerProvidedFailoverPartner); diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs index f797797758..72941a988c 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs @@ -15,6 +15,7 @@ using System.Security; using System.Threading; using System.Threading.Tasks; +using System.Transactions; using Microsoft.Data.SqlClient.Tests.Common; using Microsoft.SqlServer.TDS; using Microsoft.SqlServer.TDS.FeatureExtAck; @@ -42,6 +43,50 @@ public void ConnectionTest() connection.Open(); } + /// + /// Verifies that Enlist=false prevents automatic enlistment when a public connection is + /// opened within an ambient transaction, for both synchronous and asynchronous opens. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Open_WithEnlistDisabled_DoesNotEnlistInAmbientTransaction(bool async) + { + using TdsServer server = new(new TdsServerArguments()); + server.Start(); + string connectionString = new SqlConnectionStringBuilder + { + DataSource = $"localhost,{server.EndPoint.Port}", + Encrypt = SqlConnectionEncryptOption.Optional, + Enlist = false, + Pooling = true, + }.ConnectionString; + using SqlConnection connection = new(connectionString); + + try + { + using TransactionScope scope = new(TransactionScopeAsyncFlowOption.Enabled); + Assert.NotNull(Transaction.Current); + + if (async) + { + await connection.OpenAsync(); + } + else + { + connection.Open(); + } + + Assert.Equal(ConnectionState.Open, connection.State); + Assert.Null(connection.InnerConnection.EnlistedTransaction); + scope.Complete(); + } + finally + { + SqlConnection.ClearPool(connection); + } + } + [Fact] [PlatformSpecific(TestPlatforms.Windows)] public void IntegratedAuthConnectionTest()