Configurable retry logic can retry SqlCommands that have a transaction

[antmjones]

Describe the bug

When applied to a SqlCommand the RetryCondition within SqlRetryLogic checks the Transaction property on the SqlCommand, and returns false if the Transaction is null, indicating that the command shouldn't be retried:

https://github.com/dotnet/SqlClient/blob/v7.0.1/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/Common/SqlRetryLogic.cs#L88

However, this seems to fall foul of the issue mentioned in #1509, that is that the Transaction property gets cleared if the error causes the transaction to abort, meaning that a SqlCommand with a transaction can be retried without a transaction.

This can mean that commands that were intented to run within a transaction (e.g. may need to be rolled back) can be executed outside of any transaction in case of a transient error such as a deadlock.

To reproduce

using Microsoft.Data.SqlClient;

namespace SqlCommandRetryProviderTest;

internal class Program {
    static async Task Main(string[] args) {
        const string baseConnectionString =
            @"Data Source=(localdb)\MSSQLLocalDB;Integrated Security=True;";
        const string databaseName = "TestDb";

        CreateDatabaseAndSchema(baseConnectionString + "Initial Catalog=master;", databaseName);
        await CreateDeadlock(baseConnectionString + $"Initial Catalog={databaseName};");
    }

    private static async Task CreateDeadlock(string connectionString) {
        SqlConnection conn1 = new SqlConnection(connectionString);
        conn1.Open();

        SqlConnection conn2 = new SqlConnection(connectionString);
        conn2.Open();

        using SqlTransaction txn1 = conn1.BeginTransaction();
        using SqlTransaction txn2 = conn2.BeginTransaction();

        var commandRetryProvider = SqlConfigurableRetryFactory
            .CreateExponentialRetryProvider(new SqlRetryLogicOption {
                NumberOfTries = 2,
            });

        commandRetryProvider.Retrying += (sender, eventArgs) => {
            SqlCommand cmd = (SqlCommand)sender!;
            Console.WriteLine("SqlCommand " + cmd.GetHashCode() + " has transaction: " + cmd.Transaction);
            throw new InvalidOperationException("This shouldn't happen!");
        };

        async Task ExecuteNonQueryAsync(SqlConnection conn, SqlTransaction txn, string commandText) {
            using SqlCommand cmd = new SqlCommand(commandText, conn, txn);
            Console.WriteLine("SqlCommand " + cmd.GetHashCode() + " has transaction: " + cmd.Transaction);
            cmd.RetryLogicProvider = commandRetryProvider;
            await cmd.ExecuteNonQueryAsync();
            return;
        }

        await ExecuteNonQueryAsync(conn1, txn1, "INSERT INTO [Table1] ([Id], [Value]) VALUES (1, 1)");
        await ExecuteNonQueryAsync(conn2, txn2, "INSERT INTO [Table1] ([Id], [Value]) VALUES (2, 2)");

        Task t1 = ExecuteNonQueryAsync(conn1, txn1, "UPDATE [Table1] SET [Value] = 2 WHERE [Id] = 1");
        Task t2 = ExecuteNonQueryAsync(conn2, txn2, "UPDATE [Table1] SET [Value] = 1 WHERE [Id] = 2");

        await Task.WhenAll(t1, t2);
    }

    private static void CreateDatabaseAndSchema(string connectionString, string dbName) {
        using SqlConnection conn = new SqlConnection(connectionString);
        conn.Open();

        void ExecuteNonQuery(string commandText) {
            using SqlCommand cmd = new SqlCommand(commandText, conn);
            cmd.ExecuteNonQuery();
        }

        ExecuteNonQuery($"DROP DATABASE IF EXISTS [{dbName}]");
        ExecuteNonQuery($"CREATE DATABASE [{dbName}]");
        ExecuteNonQuery($"CREATE TABLE [{dbName}].[dbo].[Table1] ([Id] int, [Value] int)");
    }
}

Expected behavior

SqlCommands that have a Transaction set at the time of first execution should not be retried. For example, in the code above, the throw new InvalidOperationException line should not be hit.

Further technical details

Microsoft.Data.SqlClient version: 7.0.1
.NET target: 10.0.8
SQL Server version: Local DB 17.0.4025.3 (also recently observed on Azure SQL and SQL Server 2022)
Operating system: Windows 11 24H2

[github-actions]

🔍 Triage Summary

Check	Result
Issue type	Bug
Environment	All required environment details provided for investigation
Area	Area\Engineering (Reliability / configurable retry logic)
Duplicates	Potentially related: #1509 (SqlCommand retry after transaction abort can partially save data)
Regression	Not indicated (design flaw in retry guard logic)

Analysis

The SqlRetryLogic.RetryCondition in SqlRetryLogic.cs (L88) guards against retrying commands with active transactions by checking SqlCommand.Transaction != null. However, as documented in #1509, when a transient error (e.g. deadlock) aborts the transaction, SQL Server clears the Transaction property on the SqlCommand, so the null-check evaluates to false — incorrectly allowing the retry. This means commands that were originally part of a transaction can execute outside any transaction context on retry, risking data integrity violations (e.g. partial saves). The severity is P1 given the silent data corruption potential.

Next Steps

• Assign to Copilot coding agent to investigate SqlRetryLogic.cs (specifically the RetryCondition at L88) and related retry infrastructure in src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/.
• The fix should capture whether a transaction was present at the start of the first execution attempt (e.g. store a flag on first RetryCondition call) rather than checking the current Transaction property at retry time. Alternatively, the retry logic should detect a cleared/aborted transaction and throw instead of retrying.
• Review relationship with SqlCommand retry after transaction abort can partially save data to database #1509, which describes the root cause (Transaction property being cleared on abort). A coordinated fix may be appropriate.
• Add regression tests covering: deadlock scenario with a transactioned SqlCommand and retry logic attached — verify retry is not attempted when the original command had a transaction.

Note: This triage summary is auto-generated by an AI agent. The analysis and suggestions above have not been verified by a human maintainer. Please treat as preliminary guidance only.

Generated by SqlClient Issue Auto-Triage for issue #4309 · ● 2.5M · ◷