Re-issue session isolation level on TransactionScope re-enlistment (fixes #146)

[priyankatiwari08]

Summary

Fix #146 — TransactionScope ambient isolation level is silently downgraded after the first pooled connection re-checkout on Azure SQL DB.

Repro

using var scope = new TransactionScope(
    TransactionScopeOption.Required,
    new TransactionOptions { IsolationLevel = IsolationLevel.Serializable },
    TransactionScopeAsyncFlowOption.Enabled);

for (int i = 0; i < 3; i++)
{
    using var c = new SqlConnection(azureConnStringWithMaxPoolSize1);
    await c.OpenAsync();
    // DBCC USEROPTIONS:
    //   i==0 -> serializable
    //   i>=1 -> read committed snapshot   <-- bug
}

On on-prem SQL Server the level survives and all three opens report serializable. On Azure SQL DB the second and subsequent opens report the database default isolation (e.g. read committed snapshot).

Root cause

1. First Open() inside the scope enlists the connection and sends SET TRANSACTION ISOLATION LEVEL <ambient>;.
2. On Close() the physical connection is returned to the transacted pool still enlisted in the same Transaction.
3. Second Open() reuses the same physical connection. SqlInternalConnectionTds.Enlist(Transaction) sees transaction.Equals(EnlistedTransaction) and short-circuits — no SET is sent.
4. A pending sp_reset_connection_keep_transaction piggybacks on the next batch. On Azure SQL DB this reset clears the session isolation level to the database default; on on-prem SQL Server it does not, which is why the bug is Azure-only in practice.

Fix

On the short-circuit path in Enlist, when reset is pending and the new behavior switch is enabled, re-issue:

SET TRANSACTION ISOLATION LEVEL <ambient>;

mapped from Transaction.IsolationLevel. The statement is queued onto the same TDS batch as the reset, so there is no extra round trip.

Gated behind a new AppContext switch for back-compat:

• Switch.Microsoft.Data.SqlClient.UseLegacyTransactionScopeIsolationBehavior (default false).

Same back-compat pattern used by the related companion PR #4330 (UseLegacyIsolationLevelBehavior).

Files changed

• src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs — new switch.
• src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs — new re-attach branch + ReassertSessionIsolationLevel helper.
• src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/TransactionScopeIsolationReassertTest.cs — new ManualTests, gated on IsAzureServer.
• src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs + tests/UnitTests/.../LocalAppContextSwitchesTest.cs — switch wired into the RAII helper and defaults test.

Validation

End-to-end repro against both back ends with Pooling=true; Max Pool Size=1, opening 3 connections per scope.

Scenario	Conn 1	Conn 2	Conn 3
On-prem SQL Server, default	serializable	serializable	serializable
Azure SQL DB, default (fix)	serializable	serializable	serializable
Azure SQL DB, legacy switch on	serializable	read committed snapshot	read committed snapshot

Build clean on net462, net8.0, net9.0 (0 warnings, 0 errors).

Snapshot isolation is intentionally not re-asserted

Per SET TRANSACTION ISOLATION LEVEL, switching to SNAPSHOT mid-transaction causes the transaction to fail and roll back. This re-attach path always runs with the preserved transaction still open, so emitting the SET for a Snapshot scope would throw out of SqlConnection.Open(). ReassertSessionIsolationLevel returns early for Snapshot — the delegated transaction was already begun under snapshot isolation via the TM request, so there is nothing to re-assert.

Notes

• New ManualTests are gated on DataTestUtility.IsAzureServer because the bug does not manifest against on-prem SQL Server. LegacySwitch_PreservesAzureDowngradeBehavior asserts NotEqual("Serializable"), so it assumes the target database default is not serializable.
• The SET batch is an extra round trip on every pooled re-checkout inside a TransactionScope, on all back ends (the queued reset piggybacks its TDS header, so the reset itself is free). Flagging for maintainers in case this should be narrowed — e.g. gated on Azure, or deferred so the SET prefixes the user's next batch.
• Companion PR for the related but distinct cross-user pool leak: Reset session isolation level on pool return (fixes #96) #4330.

[Copilot]

Pull request overview

Fixes an Azure SQL DB-specific TransactionScope pooling regression where session isolation level can revert to the database default after transacted-pool re-checkout by re-asserting the ambient isolation level during re-enlistment.

Changes:

• Added a new AppContext switch (Switch.Microsoft.Data.SqlClient.UseLegacyTransactionScopeIsolationBehavior) to gate the new re-assert behavior.
• Updated SqlInternalConnectionTds.Enlist(Transaction) to re-issue SET TRANSACTION ISOLATION LEVEL ... on the “same transaction” short-circuit path (intended to piggyback on the pending reset).
• Added new Azure-gated ManualTests and wired them into the ManualTests project.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs	Adds re-attach logic to re-assert session isolation level when re-enlisting into the same ambient transaction.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs	Introduces a new AppContext switch to enable legacy behavior.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/TransactionScopeIsolationReassertTest.cs	Adds ManualTests validating isolation level stability across pooled re-opens inside a TransactionScope (Azure-only).
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj	Includes the new ManualTests source file in the build.

[priyankatiwari08]

Fixed in 6c57f75 and 90c58a1. LocalAppContextSwitchesHelper now captures/restores s_useLegacyTransactionScopeIsolationBehavior and exposes a UseLegacyTransactionScopeIsolationBehavior accessor, and TestDefaultAppContextSwitchValues resets and asserts the new switch. After merging main I also switched the getter to GetSwitchPropertyValue to match the accessor pattern main moved to; the defaults test passes.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

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

• The PR description states the isolation re-assert is queued onto the same batch as the pending reset with no extra round trip, but this implementation runs a synchronous TdsExecuteSQLBatch + Run during Open/Enlist, which introduces an additional server round trip on the re-attach path. Please reconcile the PR description with the actual behavior, or adjust the implementation to truly piggyback without an extra Open-time execute.

            else if (!LocalAppContextSwitches.UseLegacyTransactionScopeIsolationBehavior
                     && _parser._fResetConnection)
            {
                // Same System.Transactions transaction being re-attached to the same
                // pooled physical connection (transacted-pool re-checkout inside an
                // open TransactionScope). The queued sp_reset_connection_keep_transaction
                // does not preserve the SQL Server session isolation level on every
                // server (notably Azure SQL DB), so without re-asserting the level the
                // second and later opens inside the scope would silently run at the
                // database default. The queued reset piggybacks this batch's TDS
                // header, so the reset itself costs nothing extra, but the SET batch
                // is an additional round trip on re-checkout.
                ReassertSessionIsolationLevel(transaction.IsolationLevel);

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/TransactionScopeIsolationReassertTest.cs:19

• ManualTests in this folder are partitioned with [Trait("Set", "3")] (see TransactionTest.cs / TransactionEnlistmentTest.cs / DistributedTransactionTest.cs). This new test class is missing the trait, which can cause it to run outside the intended ManualTests set partitioning.

    public static class TransactionScopeIsolationReassertTest
    {

[codecov]

Codecov Report

❌ Patch coverage is 14.28571% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.79%. Comparing base (ee529d4) to head (90c58a1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...Data/SqlClient/Connection/SqlConnectionInternal.cs	2.70%	36 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4335      +/-   ##
==========================================
- Coverage   64.78%   62.79%   -1.99%     
==========================================
  Files         288      283       -5     
  Lines       44418    67452   +23034     
==========================================
+ Hits        28774    42358   +13584     
- Misses      15644    25094    +9450     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	62.79% <14.28%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[priyankatiwari08]

Added a design note at specs/007-session-isolation-level/design.md (identical file in #4330) explaining why this PR and #4330 are not duplicate fixes.

Short version: both bugs come from the same fact — sp_reset_connection clears the session transaction_isolation_level on Azure SQL DB but not on on-prem SQL Server — but they sit on opposite sides of it:

	#146 / this PR	#96 / #4330
Failure	Level is cleared when it should persist (silent downgrade)	Level persists when it should be cleared (leak)
Transaction state	Still open / ambient	Already completed
Who is harmed	The same caller, next Open() in the scope	An unrelated later pool consumer
Servers	Azure SQL DB only	On-prem SQL Server
API surface	TransactionScope only	SqlTransaction and TransactionScope
Code path	Enlist() — pool checkout	ResetConnection() — pool return
T-SQL emitted	SET ... <ambient level> (dynamic)	SET ... READ COMMITTED (fixed)
Direction	Re-assert session state	Scrub session state
Snapshot	Deliberately skipped (would roll back the open txn)	Reset like any other level

Neither subsumes the other: this PR only fires on the Enlist() same-transaction re-attach branch, which the #96 repro — no live transaction, often plain SqlTransaction — never reaches; and #4330 only runs on pool return and only ever writes READ COMMITTED, which is the wrong level for the ambient scope here.

The two PRs do overlap textually (same file, same switches helper, same test folder). Suggested sequencing is #4330 first, then rebase this one on top and settle the open perf question (unconditional SET vs. Azure-gated vs. deferring the SET to prefix the user's next batch).

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/TransactionScopeIsolationReassertTest.cs:18

• ManualTests are partitioned by the xUnit Set trait (see build.proj TestSetFilter). This new test class doesn’t declare a Set, so it may be skipped when CI runs specific manual test sets (e.g., Set=1/2/3). Add the same [Trait("Set", "3")] used by other transaction manual tests so these new tests reliably execute in the manual test matrix.

    public static class TransactionScopeIsolationReassertTest