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

SqlTransaction and TransactionScope leave the underlying SQL Server session with the elevated isolation level after Commit/Rollback. sp_reset_connection does not reset it, so the next user of the pooled physical connection silently inherits it.

This change tracks when a non-default isolation level was set via the TM Begin path and, on pool return, issues a 'SET TRANSACTION ISOLATION LEVEL READ COMMITTED' batch right before sp_reset_connection (piggybacked on the same TDS header, no extra round trip). On failure the connection is doomed instead of returned to the pool.

A new AppContext switch 'Switch.Microsoft.Data.SqlClient.UseLegacyIsolationLevelBehavior' preserves previous behavior for callers that depend on it (default: false).

Adds IsolationLevelLeakTest under ManualTests covering the two repro scenarios plus a negative test for the kill switch.

Author: priyankatiwari08

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/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,149 @@
+// 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 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 PooledMaxOneConnString =>
+            new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
+            {
+                Pooling = true,
+                MaxPoolSize = 1,
+                MultipleActiveResultSets = false,
+                ApplicationName = "IsoLeakTest"
+            }.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 = PooledMaxOneConnString;
+            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 = PooledMaxOneConnString;
+            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. Runs in
+        // an isolated AppDomain on .NET Framework / process boundary on .NET
+        // would be ideal, but AppContext switches set before any pool entry
+        // is created suffice when this test runs first in its own collection.
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        public static void LegacySwitch_PreservesOldLeakBehavior()
+        {
+            const string Switch = "Switch.Microsoft.Data.SqlClient.UseLegacyIsolationLevelBehavior";
+            AppContext.SetSwitch(Switch, true);
+            try
+            {
+                // Use a distinct app name so this test gets its own pool group
+                // and isn't affected by entries created by the other tests.
+                string cs = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
+                {
+                    Pooling = true,
+                    MaxPoolSize = 1,
+                    MultipleActiveResultSets = false,
+                    ApplicationName = "IsoLeakTest-Legacy"
+                }.ConnectionString;
+
+                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
+            {
+                AppContext.SetSwitch(Switch, false);
+                SqlConnection.ClearAllPools();
+            }
+        }
+
+        private static string GetIsoOnTx(SqlConnection c, SqlTransaction tx)
+        {
+            using SqlCommand cmd = new SqlCommand(GetIsoSql, c, tx);
+            return (string)cmd.ExecuteScalar();
+        }
+    }
+}