Reset session isolation level on pool return (fixes #96)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs

@@ -281,6 +281,14 @@ internal class SqlConnectionInternal : DbConnectionInternal, IDisposable
         // @TODO: Rename to match naming conventions (remove f prefix)
         private readonly bool _fResetConnection;
 
+        // Tracks whether a SQL Server session transaction isolation level
+        // change has been issued on this connection (via SqlTransaction or
+        // System.Transactions enlistment) but not yet undone. SQL Server's
+        // sp_reset_connection does not reset the session isolation level, so
+        // without compensation a pooled connection leaks the level to its
+        // next user.
+        private bool _isolationLevelDirty;
+
 
 
         // @TODO: Rename to match naming conventions
@@ -2679,6 +2687,16 @@ private void ExecuteTransaction2005(
                     internalTransaction,
                     stateObj,
                     isDelegateControlRequest);
+
+                // A successful Begin with a non-default isolation level mutates
+                // SQL Server's session transaction_isolation_level. sp_reset_connection
+                // will not undo this, so remember it and reset on deactivate.
+                if (requestType == TdsEnums.TransactionManagerRequestType.Begin &&
+                    isoLevel != TdsEnums.TransactionManagerIsolationLevel.Unspecified &&
+                    isoLevel != TdsEnums.TransactionManagerIsolationLevel.ReadCommitted)
+                {
+                    _isolationLevelDirty = true;
+                }
             }
             finally
             {
@@ -3894,6 +3912,42 @@ internal override void ResetConnection()
                 // Reset dictionary values, since calling reset will not send us env_changes.
                 CurrentDatabase = _originalDatabase;
                 _currentLanguage = _originalLanguage;
+
+                // sp_reset_connection (set up by PrepareResetConnection above) does not reset the
+                // session transaction isolation level. When we know a previous Begin changed the
+                // level, issue an explicit SET so the next user of this pooled connection starts at
+                // READ COMMITTED. The batch piggybacks the queued sp_reset_connection on its TDS
+                // header, so the round trip is shared and no separate reset packet is sent later.
+                // Gated by an app-context switch for back-compat.
+                if (_isolationLevelDirty && !LocalAppContextSwitches.UseLegacyIsolationLevelBehavior)
+                {
+                    ResetSessionIsolationLevel();
+                }
+            }
+        }
+
+        // Issues "SET TRANSACTION ISOLATION LEVEL READ COMMITTED;" on the physical state object
+        // so the next user of this pooled connection observes the default session isolation level.
+        // Called only from ResetConnection when _isolationLevelDirty is set; runs synchronously
+        // because Deactivate is a synchronous pool-return path.
+        private void ResetSessionIsolationLevel()
+        {
+            try
+            {
+                _parser.TdsExecuteSQLBatch(
+                    text: "SET TRANSACTION ISOLATION LEVEL READ COMMITTED;",
+                    timeout: 0,
+                    notificationRequest: null,
+                    stateObj: _parser._physicalStateObj,
+                    sync: true);
+                _parser.Run(RunBehavior.UntilDone, null, null, null, _parser._physicalStateObj);
+                _isolationLevelDirty = false;
+            }
+            catch (Exception e) when (ADP.IsCatchableExceptionType(e))
+            {
+                // If we cannot scrub the session state, the connection is not safe to pool.
+                // Dooming forces Deactivate's outer logic to destroy it instead of pooling it.
+                DoomThisConnection();
             }
         }
 

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs

@@ -65,6 +65,14 @@ internal static class LocalAppContextSwitches
     private const string UseLegacyFailoverAlternationOnLoginSqlErrorsString =
         "Switch.Microsoft.Data.SqlClient.UseLegacyFailoverAlternationOnLoginSqlErrors";
 
+    /// <summary>
+    /// The name of the app context switch that controls whether pooled connections
+    /// preserve the legacy behavior of leaking the SQL Server session
+    /// transaction isolation level across pool reuse.
+    /// </summary>
+    private const string UseLegacyIsolationLevelBehaviorString =
+        "Switch.Microsoft.Data.SqlClient.UseLegacyIsolationLevelBehavior";
+
     /// <summary>
     /// The name of the app context switch that controls whether to preserve
     /// legacy behavior where Timestamp/RowVersion fields return empty byte
@@ -201,6 +209,11 @@ private enum SwitchValue : byte
     /// </summary>
     private static SwitchValue s_useLegacyFailoverAlternationOnLoginSqlErrors = SwitchValue.None;
 
+    /// <summary>
+    /// The cached value of the UseLegacyIsolationLevelBehavior switch.
+    /// </summary>
+    private static SwitchValue s_useLegacyIsolationLevelBehavior = SwitchValue.None;
+
     /// <summary>
     /// The cached value of the LegacyRowVersionNullBehavior switch.
     /// </summary>
@@ -446,6 +459,22 @@ public static bool GlobalizationInvariantMode
             defaultValue: false,
             ref s_useLegacyFailoverAlternationOnLoginSqlErrors);
 
+    /// <summary>
+    /// When set to true, pooled connections preserve the legacy behavior where
+    /// the SQL Server session transaction isolation level is not reset on
+    /// return to the pool. As a result, the next user of the pooled connection
+    /// may inherit a non-default isolation level.
+    ///
+    /// The default value of this switch is false, meaning the driver will
+    /// reset the isolation level to READ COMMITTED before the next checkout
+    /// when it knows the level was changed.
+    /// </summary>
+    public static bool UseLegacyIsolationLevelBehavior =>
+        AcquireAndReturn(
+            UseLegacyIsolationLevelBehaviorString,
+            defaultValue: false,
+            ref s_useLegacyIsolationLevelBehavior);
+
     /// <summary>
     /// In System.Data.SqlClient and Microsoft.Data.SqlClient prior to 3.0.0 a
     /// field with type Timestamp/RowVersion would return an empty byte array.

src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs

@@ -46,6 +46,7 @@ public sealed class LocalAppContextSwitchesHelper : IDisposable
     #endif
     private readonly bool? _ignoreServerProvidedFailoverPartnerOriginal;
     private readonly bool? _useLegacyFailoverAlternationOnLoginSqlErrorsOriginal;
+    private readonly bool? _useLegacyIsolationLevelBehaviorOriginal;
     private readonly bool? _legacyRowVersionNullBehaviorOriginal;
     private readonly bool? _legacyVarTimeZeroScaleBehaviourOriginal;
     private readonly bool? _makeReadAsyncBlockingOriginal;
@@ -100,6 +101,8 @@ public LocalAppContextSwitchesHelper()
                 GetSwitchValue("s_ignoreServerProvidedFailoverPartner");
             _useLegacyFailoverAlternationOnLoginSqlErrorsOriginal =
                 GetSwitchValue("s_useLegacyFailoverAlternationOnLoginSqlErrors");
+            _useLegacyIsolationLevelBehaviorOriginal =
+                GetSwitchValue("s_useLegacyIsolationLevelBehavior");
             _legacyRowVersionNullBehaviorOriginal =
                 GetSwitchValue("s_legacyRowVersionNullBehavior");
             _legacyVarTimeZeroScaleBehaviourOriginal =
@@ -161,6 +164,9 @@ public void Dispose()
             SetSwitchValue(
                 "s_useLegacyFailoverAlternationOnLoginSqlErrors",
                 _useLegacyFailoverAlternationOnLoginSqlErrorsOriginal);
+            SetSwitchValue(
+                "s_useLegacyIsolationLevelBehavior",
+                _useLegacyIsolationLevelBehaviorOriginal);
             SetSwitchValue(
                 "s_legacyRowVersionNullBehavior", 
                 _legacyRowVersionNullBehaviorOriginal);
@@ -261,6 +267,15 @@ public bool? UseLegacyFailoverAlternationOnLoginSqlErrors
         set => SetSwitchValue("s_useLegacyFailoverAlternationOnLoginSqlErrors", value);
     }
 
+    /// <summary>
+    /// Get or set the UseLegacyIsolationLevelBehavior switch value.
+    /// </summary>
+    public bool? UseLegacyIsolationLevelBehavior
+    {
+        get => GetSwitchValue("s_useLegacyIsolationLevelBehavior");
+        set => SetSwitchValue("s_useLegacyIsolationLevelBehavior", value);
+    }
+
     /// <summary>
     /// Get or set the LegacyRowVersionNullBehavior switch value.
     /// </summary>

src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj

@@ -233,6 +233,7 @@
     <Compile Include="SQL\SqlSchemaInfoTest\SqlSchemaInfoTest.cs" />
     <Compile Include="SQL\SqlStatisticsTest\SqlStatisticsTest.cs" />
     <Compile Include="SQL\TransactionTest\DistributedTransactionTest.cs" />
+    <Compile Include="SQL\TransactionTest\IsolationLevelLeakTest.cs" />
     <Compile Include="SQL\TransactionTest\TransactionEnlistmentTest.cs" />
     <Compile Include="SQL\TransactionTest\TransactionTest.cs" />
     <Compile Include="SQL\UdtTest\SqlServerTypesTest.cs" />

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/IsolationLevelLeakTest.cs

@@ -0,0 +1,150 @@
+// 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.Data;
+using System.Transactions;
+using Microsoft.Data.SqlClient.Tests.Common;
+using Xunit;
+using IsolationLevel = System.Data.IsolationLevel;
+
+namespace Microsoft.Data.SqlClient.ManualTesting.Tests
+{
+    // SqlTransaction / TransactionScope used to leave the pooled connection
+    // with the elevated session isolation level. The next user of the pooled
+    // connection would silently inherit it. The fix resets the session
+    // isolation level to READ COMMITTED when the connection is returned to
+    // the pool.
+    public static class IsolationLevelLeakTest
+    {
+        private const string GetIsoSql = @"
+SELECT CASE transaction_isolation_level
+            WHEN 0 THEN 'Unspecified'
+            WHEN 1 THEN 'ReadUncommitted'
+            WHEN 2 THEN 'ReadCommitted'
+            WHEN 3 THEN 'RepeatableRead'
+            WHEN 4 THEN 'Serializable'
+            WHEN 5 THEN 'Snapshot'
+       END
+FROM sys.dm_exec_sessions WHERE session_id = @@SPID;";
+
+        private static string BuildPooledConnString(string appName) =>
+            new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
+            {
+                Pooling = true,
+                MaxPoolSize = 1,
+                MultipleActiveResultSets = false,
+                Enlist = true,
+                ApplicationName = appName
+            }.ConnectionString;
+
+        private static int GetSpid(SqlConnection c)
+        {
+            using SqlCommand cmd = new SqlCommand("SELECT @@SPID;", c);
+            return Convert.ToInt32(cmd.ExecuteScalar());
+        }
+
+        private static string GetIso(SqlConnection c)
+        {
+            using SqlCommand cmd = new SqlCommand(GetIsoSql, c);
+            return (string)cmd.ExecuteScalar();
+        }
+
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        public static void SqlTransaction_SerializableDoesNotLeakAcrossPool()
+        {
+            string cs = BuildPooledConnString("IsoLeakTest-SqlTx");
+            int spid1;
+            using (SqlConnection c = new SqlConnection(cs))
+            {
+                c.Open();
+                spid1 = GetSpid(c);
+                using SqlTransaction tx = c.BeginTransaction(IsolationLevel.Serializable);
+                Assert.Equal("Serializable", GetIsoOnTx(c, tx));
+                tx.Rollback();
+            }
+
+            using (SqlConnection c = new SqlConnection(cs))
+            {
+                c.Open();
+                Assert.Equal(spid1, GetSpid(c)); // pool reuse
+                Assert.Equal("ReadCommitted", GetIso(c));
+            }
+        }
+
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        public static void TransactionScope_SerializableDoesNotLeakAcrossPool()
+        {
+            string cs = BuildPooledConnString("IsoLeakTest-TxScope");
+            int spid1;
+            using (var scope = new TransactionScope(
+                TransactionScopeOption.RequiresNew,
+                new TransactionOptions { IsolationLevel = System.Transactions.IsolationLevel.Serializable },
+                TransactionScopeAsyncFlowOption.Enabled))
+            using (SqlConnection c = new SqlConnection(cs))
+            {
+                c.Open();
+                spid1 = GetSpid(c);
+                Assert.Equal("Serializable", GetIso(c));
+                scope.Complete();
+            }
+
+            using (SqlConnection c = new SqlConnection(cs))
+            {
+                c.Open();
+                Assert.Equal(spid1, GetSpid(c));
+                Assert.Equal("ReadCommitted", GetIso(c));
+            }
+        }
+
+        // Negative test: legacy switch ON brings the old leak back.
+        // Only meaningful on on-prem SQL Server: Azure SQL DB resets the
+        // session isolation level inside sp_reset_connection, so the leak
+        // never materializes there regardless of the switch.
+        // Uses LocalAppContextSwitchesHelper to force the cached switch value
+        // in LocalAppContextSwitches (production code reads the cache, not
+        // AppContext, after first use). The helper restores the previous
+        // value on dispose.
+        [ConditionalFact(
+            typeof(DataTestUtility),
+            nameof(DataTestUtility.AreConnStringsSetup),
+            nameof(DataTestUtility.IsNotAzureServer))]
+        public static void LegacySwitch_PreservesOldLeakBehavior()
+        {
+            using LocalAppContextSwitchesHelper switchesHelper = new();
+            switchesHelper.UseLegacyIsolationLevelBehavior = true;
+
+            // Distinct app name so this test gets its own pool group.
+            string cs = BuildPooledConnString("IsoLeakTest-Legacy");
+            try
+            {
+                int spid1;
+                using (SqlConnection c = new SqlConnection(cs))
+                {
+                    c.Open();
+                    spid1 = GetSpid(c);
+                    using SqlTransaction tx = c.BeginTransaction(IsolationLevel.Serializable);
+                    tx.Rollback();
+                }
+
+                using (SqlConnection c = new SqlConnection(cs))
+                {
+                    c.Open();
+                    Assert.Equal(spid1, GetSpid(c));
+                    Assert.Equal("Serializable", GetIso(c)); // legacy: leaks
+                }
+            }
+            finally
+            {
+                SqlConnection.ClearAllPools();
+            }
+        }
+
+        private static string GetIsoOnTx(SqlConnection c, SqlTransaction tx)
+        {
+            using SqlCommand cmd = new SqlCommand(GetIsoSql, c, tx);
+            return (string)cmd.ExecuteScalar();
+        }
+    }
+}