SqlClient: System.Data.SqlClient.SqlConnection does not roll back open transaction on dispose

SQL Server version is 2014 if it matters. Trivial reproduction causes timeout unless ;pooling=false is appended to the connection string.

    using System;
    using System.Data;
    using System.Data.SqlClient;

    /*
    CREATE TABLE Scratch (ID INTEGER NOT NULL);
    INSERT INTO Scratch VALUES (1);
    */

    namespace ConsoleApplication
    {
        public class Program
        {
            public static void Main(string[] args)
            {
                if (args.Length == 0) {
                    Console.WriteLine("Usage: dotnet disposeconnection.dll connectionstring");
                    return ;
                }
                using (var c1 = new SqlConnection(args[0]))
                {
                    c1.Open();
                    using (var c2 = new SqlConnection(args[0]))
                    {
                        c2.Open();
                        Exec(c2, "BEGIN TRANSACTION;");
                        Exec(c2, "UPDATE Scratch SET ID = 2;");
                    }
                    Exec(c1, "UPDATE Scratch SET ID=3;");
                }
            }

            private static void Exec(SqlConnection c, string s)
            {
                using (var m = c.CreateCommand())
                {
                    m.CommandText = s;
                    m.ExecuteNonQuery();
                }
            }
        }
    }

We got here through a much larger chunk of code handling thrown exceptions. We have no idea what the connection state is and it might have an executing data reader on it so we have no good way of passing the rollback command ourselves.

About this issue

Original URL
State: closed
Created 8 years ago
Comments: 18 (2 by maintainers)

Most upvoted comments

This issue happens because unmanaged transaction is not rolled back before the connection moves back the pool. That means this error only happens when user initiates unmanaged transaction by executing BEGIN TRANSACTION command manually in code, and the transaction is not finished properly before the connection moves back to the pool.

Recommended (managed) way of initiating transaction is using SqlTransaction object by beginning with SqlConnection.BeginTransaction() as following:

using (var sqlConnection = new SqlConnection(connString))
{
    sqlConnection.Open();

    using (var trans = sqlConnection.BeginTransaction())
    {
        using (var cmd = new SqlCommand("UPDATE Scratch SET ID=1;", sqlConnection, trans))
        {
            cmd.ExecuteNonQuery();
        }
    }
}

In this case, incomplete transaction is rolled back automatically when SqlTransaction object is disposed in out of using scope. Even if user does not specify using block for transaction object, SqlTransaction is disposed by GC eventually, and rollback for incomplete transaction happens automatically.

But if user initiates transaction by executing SQL command BEGIN TRANSACTION manually (unmanaged) :

using (var sqlConnection = new SqlConnection(connString))
{
    sqlConnection.Open();
    
    using (var cmd = new SqlCommand("BEGIN TRANSACTION;", sqlConnection, trans))
    {
        cmd.ExecuteNonQuery();
    }
    
    using (var cmd = new SqlCommand("UPDATE Scratch SET ID=1;", sqlConnection, trans))
    {
        cmd.ExecuteNonQuery();
    }
}

In this case, the transaction is never rolled back when the connection moves back to the pool because we do not have control for transaction in runtime since the transaction runs without SqlTransaction object we can access. We cannot even know if it is in the middle of transaction or not in runtime when the connection moves back to the pool.

One possible way to resolve this issue is putting following line in SqlConnection.CloseInnerConnection() method, which rolls back whatever remains as incomplete transaction in SqlConnection object before the connection moves back to pool :

this.BeginTransaction().Dispose();

This line resolves the issue because rollback occurs when Dispose() is called. However, we decided not to do this as following reasons:

This produces extra rollback request to server every time pooled connection is closed, which will lead significant performance decrease.
It is more consistent to keep it unmanaged, and let user finish the transaction properly before closing connection if the transaction is initiated by manual SQL command by user.

geleems on Apr 28, 2017

The why behind this code is hard to understand due to reduction. The actual problem is the second connection got sent back to the connection pool with an open transaction. In the real code, there was a thrown exception after the Exec(“UPDATE …”); line, and another thread picked up the connection with an open transaction.

The reason there’s no rollback in a finally block is because if a data reader were open, trying to execute the rollback would itself throw.

jhudsoncedaron on Apr 27, 2017