Sqlclient: SqlTransaction and TransactionScope leak isolation level

[madelson]

The TransactionScope class seems to "leak" it's isolation level to future queries on the same (pooled) connection.

I would expect the isolation level of the connection to be restored when the transaction ends or is disposed.

Here is a snippet which reproduces the behavior:

```c#
SqlConnection.ClearAllPools();
var conn = new SqlConnection(new SqlConnectionStringBuilder { DataSource = @".\sqlexpress", IntegratedSecurity = true }.ConnectionString);

Action printIsolationLevel = () =>
{
var cmd = conn.CreateCommand();
cmd.CommandText = @"SELECT CASE transaction_isolation_level
WHEN 0 THEN 'Unspecified'
WHEN 1 THEN 'ReadUncommitted'
WHEN 2 THEN 'ReadCommitted'
WHEN 3 THEN 'Repeatable'
WHEN 4 THEN 'Serializable'
WHEN 5 THEN 'Snapshot' END AS TRANSACTION_ISOLATION_LEVEL
FROM sys.dm_exec_sessions
where session_id = @@SPID";
Console.WriteLine(cmd.ExecuteScalar());
};

conn.Open();
printIsolationLevel(); // "ReadCommitted"
conn.Close();

using (var scope = new TransactionScope(
TransactionScopeOption.RequiresNew,
new TransactionOptions { IsolationLevel = System.Transactions.IsolationLevel.Serializable }))
{
conn.Open();
printIsolationLevel(); // "Serializable"
conn.Close();
}

conn.Open();
printIsolationLevel(); // "Serializable" !!?
conn.Close();
```

[danmosemsft]

TransactionsScope is in S.Transactions. Of course I don't know where the bug is..

[madelson]

Further investigations suggest that this may be a problem with SqlTransaction as well.

[jimcarley]

This may be a Sql connection pooling issue causing retention of the isolation level that was obtained from the transaction that was created and completed by the TransactionScope.

From: Dan Moseley [mailto:[email protected]]
Sent: Wednesday, April 12, 2017 5:19 PM
To: dotnet/corefx [e-mail]
Cc: Subscribed [e-mail]
Subject: Re: [dotnet/corefx] TransactionScope leaks isolation level (#18277)

TransactionsScope is in S.Transactions. Of course I don't know where the bug is..

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fcorefx%2Fissues%2F18277%23issuecomment-293742713&data=02%7C01%7Cjcarley%40microsoft.com%7Cdc1af23791ba4c8bfb2a08d48202bc91%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276395643083008&sdata=dFoWFD%2BpU7st62p6b%2BHwT%2BR%2F6cQwe9sZTjTslBkrpzU%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAMw2-Gl4lfajvOBedRdZ-ej2kfkpBXdLks5rvWoKgaJpZM4M7d7l&data=02%7C01%7Cjcarley%40microsoft.com%7Cdc1af23791ba4c8bfb2a08d48202bc91%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276395643083008&sdata=E%2FxOU%2BlEEKUw2%2FdjCGqp6Z4KPLE5Q0NCYYhF3PqrqcU%3D&reserved=0.

[madelson]

The fact that the isolation level change goes on to pollute the connection pool makes the much more worrisome. However, even on a single connection we can see weird behavior where the transaction isolation level leaks out of the transaction scope:

SqlConnection.ClearAllPools();
var conn = new SqlConnection(new SqlConnectionStringBuilder { DataSource = @".\sqlexpress", IntegratedSecurity = true }.ConnectionString);

Action<SqlTransaction> printIsolationLevel = (SqlTransaction transaction) =>
{
    var cmd = conn.CreateCommand();
    cmd.Transaction = transaction;
    cmd.CommandText = @"SELECT CASE transaction_isolation_level 
                        WHEN 0 THEN 'Unspecified' 
                        WHEN 1 THEN 'ReadUncommitted' 
                        WHEN 2 THEN 'ReadCommitted' 
                        WHEN 3 THEN 'Repeatable' 
                        WHEN 4 THEN 'Serializable' 
                        WHEN 5 THEN 'Snapshot' END AS TRANSACTION_ISOLATION_LEVEL 
                        FROM sys.dm_exec_sessions 
                        where session_id = @@SPID";
    Console.WriteLine(cmd.ExecuteScalar());
};

conn.Open();

printIsolationLevel(null); // ReadCommitted

using (var transaction = conn.BeginTransaction(System.Data.IsolationLevel.Serializable))
{
    printIsolationLevel(transaction); // Serializable
}

printIsolationLevel(null); // Serializable

conn.Close();

[jimcarley]

@saurabh500 Including you because it looks like you were the last person to touch SqlConnection.

This looks like a question for SqlConnection or SqlCommand.

[danmosemsft]

@corivera ?

[saurabh500]

Right now SqlClient doesn't support TransactionScope in .Net Core .
We are looking into SqlTransaction right now.

[geleems]

I performed some investigation, and realized this behavior is by design.
Here is my test code:

public static void Test()
{
    string connString = "Server=tcp:<hostname>,1433;User ID=<id>;Password=<pw>;Connect Timeout=600;";

    SqlConnection.ClearAllPools();
    var conn = new SqlConnection(new SqlConnectionStringBuilder(connString).ConnectionString);

    Action<SqlTransaction> printIsolationLevel = (SqlTransaction transaction) =>
    {
        var cmd = conn.CreateCommand();
        cmd.Transaction = transaction;
        cmd.CommandText = @"SELECT CASE transaction_isolation_level 
                WHEN 0 THEN 'Unspecified' 
                WHEN 1 THEN 'ReadUncommitted' 
                WHEN 2 THEN 'ReadCommitted' 
                WHEN 3 THEN 'Repeatable' 
                WHEN 4 THEN 'Serializable' 
                WHEN 5 THEN 'Snapshot' END AS TRANSACTION_ISOLATION_LEVEL 
                FROM sys.dm_exec_sessions 
                where session_id = @@SPID";
        Console.WriteLine(cmd.ExecuteScalar());
    };

    conn.Open();

    printIsolationLevel(null); // ReadCommitted

    using (var transaction = conn.BeginTransaction(System.Data.IsolationLevel.Serializable))
    {
        printIsolationLevel(transaction); // Serializable
    }

    printIsolationLevel(null); // Serializable

    using (var transaction = conn.BeginTransaction(System.Data.IsolationLevel.ReadUncommitted))
    {
        printIsolationLevel(transaction); // ReadUncommitted
    }

    printIsolationLevel(null); // ReadUncommitted

    conn.Close();
}

Default isolation level is ReadCommitted, and it is replaced to new isolation level value when a transaction object is created. The isolation level value remains in connection of server side (client side does not store anything after transaction is disposed) even after transaction is finished because there is no need to reset it to default since isolation level is only consumed by transaction, and the value will be replaced to new value anyway when new transaction is started.

This behavior is intended and described in TDS protocol design document (https://msdn.microsoft.com/en-us/library/dd339887.aspx). When a transaction object is disposed, it internally executes ROLLBACK, which sends TM_ROLLBACK_XACT request to server.

Detail payload of the TM_ROLLBACK_XACT request is shown below:

 fBeginXact         =   BIT

 XACT_FLAGS         =   fBeginXact
                        7FRESERVEDBIT

 ISOLATION_LEVEL    =   BYTE

 XACT_NAME          =   B_VARBYTE
 BEGIN_XACT_NAME    =   B_VARBYTE

 RequestPayload     =   XACT_NAME
                        XACT_FLAGS
                        [ISOLATION_LEVEL
                        BEGIN_XACT_NAME]

If fBeginXact is 1, then ISOLATION_LEVEL can specify the isolation level to use to start the new transaction, according to the definition at the end of this table. If fBeginXact is 0, then ISOLATION_LEVEL SHOULD NOT be present.

fBeginXact bit is obviously set to 0 when transaction object is disposed, and RequestPayload does not have ISOLATION_LEVEL value intentionally.

You can see this payload implementation at:
https://github.com/dotnet/corefx/blob/master/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs#L6374

ISOLATION_LEVEL
Value: 0x00
Description: No isolation level change requested. Use current.

According to the spec document, ROLLBACK request does not change isolation level intentionally, and let it use current. So, this behavior is intended rather than a bug.

[madelson]

@geleems it isn't true that isolation level only affects transaction. As long as SQL is in autocommit mode (the default), each statement executes in an implicit transaction using the current isolation level. This is what caused me to report the bug.

This is particularly problematic because of connection pooling, since the isolation levels can leak and affect any other part of the application.

[geleems]

@madelson

since isolation level is only consumed by transaction

I meant isolation level parameter is only consumed by method that creates transaction, which is SqlConnection.BeginTransaction(). Once isolation level is set to certain value, it will remain at that setting through the session as intended until you change it. If you want to just change the isolation level setting, and don't want transaction, you can do as following:

SqlConnection.BeginTransaction(IsolationLevel.ReadCommitted).Dispose();

However, I identified it is a problem that isolation level is not cleaned up when connection moves back to pool.

[madelson]

@geleems I agree that the behavior with pooling represents the biggest problem here; that's what burned us.

Maybe it's a separate issue, but I do think that most callers to the API would expect the isolation level to reset on dispose despite the documentation that says otherwise. It seems weird that the recommended usage pattern would be to follow each transaction with another empty transaction just to get reasonable behavior. Much like the scoping of the foreach var (which changed in C#5), this seems like it could be one of those cases where despite the current behavior being intentional it is so confusing that it might be worth changing.

[madelson]

Here's a link with some additional context on this issue: https://social.msdn.microsoft.com/Forums/sqlserver/en-US/916b3d8a-c464-4ad5-8901-6f845a2a3447/sql-server-2014-reseting-isolation-level?forum=sqldatabaseengine

[divega]

As recently announced in the .NET Blog, focus on new SqlClient features an improvements is moving to the new Microsoft.Data.SqlClient package. For this reason, we are moving this issue to the new repo at https://github.com/dotnet/SqlClient. We will still use https://github.com/dotnet/corefx to track issues on other providers like System.Data.Odbc and System.Data.OleDB, and general ADO.NET and .NET data access issues.

[madelson]

Thanks @divega . Hopefully this new package will provide a chance to fix wonky behaviors like this one!

[nycdotnet]

This behavior is surprising and the sort of thing that is hard to notice in happy-path testing, but causes issues in prod. The way we've worked around it is to change the application name for any connections that opt-in to use snapshot isolation. This has the practical effect of ADO.NET creating two connection pools (my-service and my-service-snapshot), so when the isolation mode sticks to the connections in the snapshot pool it doesn't hurt anything. It also helps with troubleshooting on the server-side because we can see which isolation mode the application was intending to use.