Reset session isolation level on pool return (fixes #96)#4330
Open
priyankatiwari08 wants to merge 1 commit into
Open
Reset session isolation level on pool return (fixes #96)#4330priyankatiwari08 wants to merge 1 commit into
priyankatiwari08 wants to merge 1 commit into
Conversation
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.
priyankatiwari08
force-pushed
the
feature/sqltransaction-isolation-leak
branch
from
4e8ed47 to
9f67fb1
Compare
Comment on lines
+31
to
+38
| private static string PooledMaxOneConnString => | ||
| new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) | ||
| { | ||
| Pooling = true, | ||
| MaxPoolSize = 1, | ||
| MultipleActiveResultSets = false, | ||
| ApplicationName = "IsoLeakTest" | ||
| }.ConnectionString; |
Comment on lines
+106
to
+109
| const string Switch = "Switch.Microsoft.Data.SqlClient.UseLegacyIsolationLevelBehavior"; | ||
| AppContext.SetSwitch(Switch, true); | ||
| try | ||
| { |
Comment on lines
+136
to
+140
| finally | ||
| { | ||
| AppContext.SetSwitch(Switch, false); | ||
| SqlConnection.ClearAllPools(); | ||
| } |
apoorvdeshmukh
requested changes
Jun 1, 2026
Contributor
apoorvdeshmukh
left a comment
apoorvdeshmukh
left a comment
There was a problem hiding this comment.
Can you look into this test failure?
Seems related to the changes
github-project-automation
Bot
moved this from To triage
to Waiting for customer
in SqlClient Board
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #96.
Problem
SqlTransactionandTransactionScopeleave the underlying SQL Serversession with the elevated isolation level after Commit/Rollback. Because
sp_reset_connectiondoes not reset the session isolation level, the nextuser of the pooled physical connection silently inherits it.
Repro
using var c = new SqlConnection(cs); c.Open();using var tx = c.BeginTransaction(IsolationLevel.Serializable); tx.Rollback();sys.dm_exec_sessions.transaction_isolation_levelis still 4 (Serializable).Reproduced on 5.2.2 and 7.0.1 against SQL Server, on both net8.0 and net472.
Fix
SqlInternalConnectionTdswhen a non-default isolation level was set via the TMBeginpath (_isolationLevelDirty).ResetConnection()(called fromDeactivateon pool return), if dirty, issueSET TRANSACTION ISOLATION LEVEL READ COMMITTED;viaTdsExecuteSQLBatch. The batch piggybacks the queuedsp_reset_connectionbyte on the same TDS header, so there is no extra round trip vs. the legacy reset.Kill switch
Switch.Microsoft.Data.SqlClient.UseLegacyIsolationLevelBehavior(defaultfalse) restores the previous behavior.Tests
Adds
IsolationLevelLeakTestunderManualTests/SQL/TransactionTest/:SqlTransaction_SerializableDoesNotLeakAcrossPoolTransactionScope_SerializableDoesNotLeakAcrossPoolLegacySwitch_PreservesOldLeakBehavior(negative test)Validation
Built clean for net462 / net8.0 / net9.0. Standalone repro run end-to-end against local SQL Server:
[After]isolation is nowReadCommittedon both scenarios with the same pooled SPID. With the legacy switch enabled,[After]isSerializable(kill-switch verified).