SqlTransaction.Dispose() throws NullReferenceException from SqlInternalTransaction.Rollback() when it races the background close of a broken connection
Maintainer thường phản hồi trong vòng 2 ngày
Đánh giá
Issue này chưa được đánh giá.
Mô tả
Describe the bug
A command can fail with a fatal error while it is pending asynchronously inside a local transaction. Two examples are a network failure, and a command timeout whose attention the server never acknowledges (an unresponsive server). Disposing the transaction straight afterwards, in a using block or an ORM closing its data context, can then throw:
Exception message: Object reference not set to an instance of an object.
Stack trace:
at Microsoft.Data.SqlClient.SqlInternalTransaction.Rollback()
at Microsoft.Data.SqlClient.SqlInternalTransaction.Dispose(Boolean disposing)
at Microsoft.Data.SqlClient.SqlInternalTransaction.Dispose()
at Microsoft.Data.SqlClient.SqlTransaction.Dispose(Boolean disposing)
Or, when the race goes the other way:
Exception message: Invalid operation. The connection is closed.
Stack trace:
at Microsoft.Data.SqlClient.SqlInternalTransaction.Rollback()
at Microsoft.Data.SqlClient.SqlInternalTransaction.Dispose(Boolean disposing)
...
Either exception replaces the original SqlException (the timeout or the transport error) that the caller was handling.
We hit the NullReferenceException in production after query timeouts against a SQL Server that had stopped responding. The caller was linq2db's DataConnection.CloseAsync() → DbTransaction.DisposeAsync(). In that trace the frames between Rollback() and the caller are inlined.
Cause
On a fatal error the driver faults the pending operation first, and closes the connection afterwards on another thread:
- Network failure:
TdsParser.ThrowExceptionAndWarning(..., asyncClose: true)completes the task with the exception, then runsSqlConnection.Close()fromTask.Factory.StartNew. - Unacknowledged timeout: the
Task.Delay(AttentionTimeoutSeconds * 1000).ContinueWith(...)inTdsParserStateObject.OnTimeoutCorecallssource.TrySetException(...), then_parser.Connection.BreakConnection(), which callsSqlConnection.Close().
So the caller's dispose runs concurrently with that close. SqlInternalTransaction.Dispose(bool) checks _innerConnection != null and calls Rollback(). Rollback() then reads _innerConnection again, first _innerConnection.IsLockedForBulkCopy (!_parser.MARSOn && ...), then ValidateConnectionForExecute, without the parser lock.
The close zombies the transaction (_innerConnection = null) and then disposes the internal connection (_parser = null). If that happens between the check and the dereference, the result is a NullReferenceException. If the connection is broken but the close has not run yet, ValidateConnectionForExecute throws ClosedConnectionError. Both exceptions are raised outside the try whose catch honours _disposing, so they escape Dispose().
The code is the same on main. #2879 and #1121 (NullReferenceException in GetServerTransactionLevel) look like the same family: _innerConnection cleared by a concurrent close while a transaction operation runs.
To reproduce
The program below runs a pending ExecuteScalarAsync inside BeginTransaction() through a TCP proxy. It breaks the connection, either with a reset or by freezing the proxy, then disposes the transaction immediately. The race window is a few instructions wide, so an EventListener on Microsoft.Data.SqlClient.EventSource sleeps on the disposing thread when the SqlInternalTransaction.Rollback | API scope event fires. That event is emitted after the null check in Dispose(bool) and before the dereference. No SqlClient code is modified. In production the same delay comes from thread-pool scheduling.
Results against SQL Server 2016 on localhost, .NET 10, Windows:
| Version | mode | hold | Outcome |
|---|---|---|---|
| 7.0.0 | reset | 0 ms | 9/10 InvalidOperationException: … connection is closed |
| 7.0.0 | reset | 300 ms | 7/10 NullReferenceException in Rollback() |
| 7.1.1 | reset | 0 ms | 4/10 InvalidOperationException: … connection is closed |
| 7.1.1 | reset | 300 ms | 2/10 NullReferenceException in Rollback() |
| 7.0.0 | freeze (timeout) | 0 ms | 2/8 InvalidOperationException: … connection is closed |
| 7.0.0 | freeze (timeout) | 300 ms | 2/8 NullReferenceException in Rollback() |
The remaining runs are ones where the background close finished before Dispose() started, so the dispose skipped the rollback. Both UseCompatibilityAsyncBehaviour / UseCompatibilityProcessSni settings reproduce it.
Repro.csproj
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net10.0</TargetFramework>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Data.SqlClient" Version="7.1.1" />
</ItemGroup>
</Project>
Program.cs
using System.Collections.Concurrent;
using System.Diagnostics.Tracing;
using System.Net;
using System.Net.Sockets;
using Microsoft.Data.SqlClient;
// Repro: SqlTransaction.Dispose() throws NullReferenceException from SqlInternalTransaction.Rollback() (or
// InvalidOperationException "connection is closed") when it races the background close of a connection that
// broke under a pending async command.
//
// The EventListener below sleeps on the disposing thread when SqlClient writes the
// "SqlInternalTransaction.Rollback | API" trace event, which is emitted after Dispose's null check and before
// Rollback's dereference. That widens the race window; no SqlClient code is changed. holdMs=0 shows the race
// without the hold.
//
// Usage: dotnet run -- "<connection string>" [holdMs=300] [iterations=10] [mode=reset|freeze] [pauseMs=0] [compat=false]
// mode reset: TCP reset while the command is pending
// mode freeze: the server stops responding; CommandTimeout (1 s) expires, the attention is never
// acknowledged, and after the 5 s attention timeout SqlClient faults the task, then closes the connection
// pauseMs: random 0..pauseMs delay between the failure and Dispose
// compat: value of both UseCompatibilityAsyncBehaviour and UseCompatibilityProcessSni
string target = args[0];
int holdMs = args.Length > 1 ? int.Parse(args[1]) : 300;
int iterations = args.Length > 2 ? int.Parse(args[2]) : 10;
string mode = args.Length > 3 ? args[3] : "reset";
int pauseMs = args.Length > 4 ? int.Parse(args[4]) : 0;
bool compat = args.Length > 5 && bool.Parse(args[5]);
AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.UseCompatibilityAsyncBehaviour", compat);
AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.UseCompatibilityProcessSni", compat);
var random = new Random(1);
var targetBuilder = new SqlConnectionStringBuilder(target);
string[] hostPort = targetBuilder.DataSource.Split(',');
using var proxy = new ResettingProxy(hostPort[0], hostPort.Length > 1 ? int.Parse(hostPort[1]) : 1433);
proxy.Start();
var viaProxy = new SqlConnectionStringBuilder(target) { DataSource = $"127.0.0.1,{proxy.Port}", Pooling = false };
using var hold = new RollbackHold();
var outcomes = new ConcurrentDictionary<string, int>();
string? firstNre = null;
for (int i = 0; i < iterations; i++)
{
var connection = new SqlConnection(viaProxy.ConnectionString);
await connection.OpenAsync();
var transaction = connection.BeginTransaction();
var pending = Task.Run(async () =>
{
using var command = new SqlCommand("WAITFOR DELAY '00:00:30'; SELECT 1", connection, transaction) { CommandTimeout = 1 };
await command.ExecuteScalarAsync();
});
await Task.Delay(200);
if (mode == "freeze")
{
proxy.Frozen = true; // the server stops responding, the socket stays open
}
else
{
proxy.ResetAll(); // the network drops while the async command is pending
}
string operation;
try { await pending; operation = "no error"; }
catch (Exception ex) { operation = ex.GetType().Name + (ex is SqlException se ? $" {se.Number}" : ""); }
if (pauseMs > 0)
{
await Task.Delay(random.Next(0, pauseMs));
}
// Dispose the transaction straight after the failure, as a using block or an ORM does.
RollbackHold.HoldMs = holdMs;
string dispose;
try { transaction.Dispose(); dispose = "ok"; }
catch (Exception ex)
{
dispose = $"{ex.GetType().Name}: {ex.Message}";
if (ex is NullReferenceException) firstNre ??= ex.ToString();
}
finally { RollbackHold.HoldMs = 0; }
connection.Dispose();
proxy.ResetAll();
proxy.Frozen = false;
outcomes.AddOrUpdate($"command: {operation}; transaction.Dispose(): {dispose}", 1, (_, n) => n + 1);
}
var version = typeof(SqlConnection).Assembly
.GetCustomAttributes(typeof(System.Reflection.AssemblyInformationalVersionAttribute), false)
.OfType<System.Reflection.AssemblyInformationalVersionAttribute>().First().InformationalVersion;
Console.WriteLine($"Microsoft.Data.SqlClient {version}; holdMs={holdMs}");
foreach (var (outcome, count) in outcomes) Console.WriteLine($"{count,4} x {outcome}");
if (firstNre != null) Console.WriteLine(firstNre);
internal sealed class RollbackHold : EventListener
{
public static volatile int HoldMs;
protected override void OnEventSourceCreated(EventSource source)
{
if (source.Name == "Microsoft.Data.SqlClient.EventSource")
{
EnableEvents(source, EventLevel.Verbose, EventKeywords.All);
}
}
protected override void OnEventWritten(EventWrittenEventArgs e)
{
if (HoldMs > 0 && e.Payload is { Count: > 0 } p && p[0] is string s
&& s.StartsWith("SqlInternalTransaction.Rollback | API", StringComparison.Ordinal))
{
Thread.Sleep(HoldMs);
}
}
}
// Forwards TCP to the server; ResetAll() aborts every forwarded connection with RST.
internal sealed class ResettingProxy(string host, int port) : IDisposable
{
private readonly TcpListener _listener = new(IPAddress.Loopback, 0);
private readonly ConcurrentDictionary<int, (TcpClient Client, TcpClient Server)> _live = new();
private int _ids;
public volatile bool Frozen;
public int Port => ((IPEndPoint)_listener.LocalEndpoint).Port;
public void Start()
{
_listener.Start();
_ = Task.Run(async () =>
{
while (true)
{
TcpClient client;
try { client = await _listener.AcceptTcpClientAsync(); }
catch (Exception) { return; }
_ = Task.Run(() => Forward(client));
}
});
}
private async Task Forward(TcpClient client)
{
int id = Interlocked.Increment(ref _ids);
var server = new TcpClient();
try
{
await server.ConnectAsync(host, port);
_live[id] = (client, server);
await Task.WhenAny(Pump(client.GetStream(), server.GetStream()), Pump(server.GetStream(), client.GetStream()));
}
catch (Exception) { }
finally
{
_live.TryRemove(id, out _);
client.Close();
server.Close();
}
}
private async Task Pump(NetworkStream from, NetworkStream to)
{
var buffer = new byte[16384];
try
{
int n;
while ((n = await from.ReadAsync(buffer)) > 0)
{
while (Frozen) await Task.Delay(10);
await to.WriteAsync(buffer.AsMemory(0, n));
}
}
catch (Exception) { }
}
public void ResetAll()
{
foreach (var id in _live.Keys)
{
if (_live.TryRemove(id, out var pair))
{
try { pair.Client.Client.LingerState = new LingerOption(true, 0); pair.Client.Close(); } catch (Exception) { }
try { pair.Server.Close(); } catch (Exception) { }
}
}
}
public void Dispose() => _listener.Stop();
}
Expected behavior
SqlTransaction.Dispose() does not throw because the connection broke. Nothing can be rolled back on a broken connection, the server ends the transaction with the session, and _disposing exists so that dispose-time rollback failures are not thrown.
Further technical details
Microsoft.Data.SqlClient version: 7.0.0 (production), 7.1.1 and main (reproduced)
.NET target: .NET 10
SQL Server version: SQL Server 2019 (production), SQL Server 2016 (repro)
Operating system: Windows Server (production), Windows 10 (repro)
Additional context
I have a fix with deterministic regression tests and will open a PR for it.
- Ngôn ngữ chính
- C#
- Star
- 1k
- Fork
- 352
- Merge trung bình
- 3 ngày 21 giờ
- Pull request đã merge (30 ngày)
- 112
Chuẩn bị môi trường
Khởi chạy dev container của dự án ngay trên trình duyệt, bằng tài khoản GitHub của bạn.
- Không có Dockerfile hay tệp Docker Compose
- Có mẫu pull request
- Đọc hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của dotnet/SqlClient
-
Sync ExecuteReader on an STA thread deadlocks against a StateChange handler that marshals to the UI thread (OpenAsyncRetry path)Có thể làm lại được @mdaigle đã nhận 50 ngày trước và không có pull request nào đang mở. Đang mởExternal :link:
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 64/100
dotnet/SqlClient#4534 · 2 bình luận · 1 người được giao ·
Maintainer thường phản hồi trong vòng 2 ngày
-
Tests | Additional test casesĐang mởArea\Tests
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 68/100
dotnet/SqlClient#4377 · 2 bình luận ·
Maintainer thường phản hồi trong vòng 2 ngày
-
Server error tokens are lost when SQL Server aborts the connection before the driver has posted its read (bare transport error instead of the server's error)Có thể đã có người làm @poizan42 đã nhận 3 ngày trước. Đang mở
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 25/100
dotnet/SqlClient#4994 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 2 ngày
-
A major performance regression from when you upgraded 5.2.2 to 7.1.1Có thể đã có người làm @cheenamalhotra đã nhận 4 ngày trước. Đang mởPerformance :chart_with_upwards_trend: Regression :boom:
dotnet/SqlClient#4985 · 3 bình luận · 1 người được giao ·
Maintainer thường phản hồi trong vòng 2 ngày
-
Suspected: async MARS command can lose its own fatal error, and sync MARS Read waits for CommandTimeout, after another session tears down the connection (managed SNI)Có thể đã có người làm Có pull request liên kết đang mở hoặc đã được merge. Đang mởTriage Needed :new:
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 15/100
Maintainer thường phản hồi trong vòng 2 ngày
Tất cả issue của dotnet/SqlClient
Issue tương tự
-
[P2] Interpreter: Array.prototype.findLastIndex starts at the wrong index for long array-likesĐang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 83/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 68/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 82/100
RicoSuter/Namotion.Interceptor#918 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
area-integrations
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
Maintainer thường phản hồi trong vòng 2 ngày
-
Sample solution restore fails with NU1109 (Microsoft.Extensions.Logging.Abstractions downgrade)Đang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 82/100
ardalis/CleanArchitecture#1150 ·
Maintainer thường phản hồi trong vòng 1 ngày