Skip to content

Commit 0322d44

Browse files
authored
Preserve distributed transactions on pooled connection reset (#3019)
1 parent 264c9c0 commit 0322d44

File tree

6 files changed

+194
-55
lines changed

6 files changed

+194
-55
lines changed

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs

+5-5
Original file line numberDiff line numberDiff line change
@@ -946,11 +946,11 @@ private void ResetConnection()
946946

947947
if (_fResetConnection)
948948
{
949-
// Ensure we are either going against 2000, or we are not enlisted in a
950-
// distributed transaction - otherwise don't reset!
951-
// Prepare the parser for the connection reset - the next time a trip
952-
// to the server is made.
953-
_parser.PrepareResetConnection(IsTransactionRoot && !IsNonPoolableTransactionRoot);
949+
// Pooled connections that are enlisted in a transaction must have their transaction
950+
// preserved when reseting the connection state. Otherwise, future uses of the connection
951+
// from the pool will execute outside of the transaction, in auto-commit mode.
952+
// https://github.com/dotnet/SqlClient/issues/2970
953+
_parser.PrepareResetConnection(EnlistedTransaction is not null && Pool is not null);
954954

955955
// Reset dictionary values, since calling reset will not send us env_changes.
956956
CurrentDatabase = _originalDatabase;

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs

+5-3
Original file line numberDiff line numberDiff line change
@@ -1075,9 +1075,11 @@ private void ResetConnection()
10751075
// distributed transaction - otherwise don't reset!
10761076
if (Is2000)
10771077
{
1078-
// Prepare the parser for the connection reset - the next time a trip
1079-
// to the server is made.
1080-
_parser.PrepareResetConnection(IsTransactionRoot && !IsNonPoolableTransactionRoot);
1078+
// Pooled connections that are enlisted in a transaction must have their transaction
1079+
// preserved when reseting the connection state. Otherwise, future uses of the connection
1080+
// from the pool will execute outside of the transaction, in auto-commit mode.
1081+
// https://github.com/dotnet/SqlClient/issues/2970
1082+
_parser.PrepareResetConnection(EnlistedTransaction is not null && Pool is not null);
10811083
}
10821084
else if (!IsEnlistedInTransaction)
10831085
{

src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionPool.cs

+2
Original file line numberDiff line numberDiff line change
@@ -1805,6 +1805,8 @@ internal void PutObjectFromTransactedPool(DbConnectionInternal obj)
18051805
Debug.Assert(obj != null, "null pooledObject?");
18061806
Debug.Assert(obj.EnlistedTransaction == null, "pooledObject is still enlisted?");
18071807

1808+
obj.DeactivateConnection();
1809+
18081810
// called by the transacted connection pool , once it's removed the
18091811
// connection from it's list. We put the connection back in general
18101812
// circulation.

src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj

+1-1
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@
200200
<Compile Include="SQL\SqlNotificationTest\SqlNotificationTest.cs" />
201201
<Compile Include="SQL\SqlSchemaInfoTest\SqlSchemaInfoTest.cs" />
202202
<Compile Include="SQL\SqlStatisticsTest\SqlStatisticsTest.cs" />
203-
<Compile Include="SQL\TransactionTest\DistributedTransactionTest.cs" />
203+
<Compile Include="SQL\TransactionTest\DistributedTransactionTest.Windows.cs" />
204204
<Compile Include="SQL\TransactionTest\TransactionTest.cs" />
205205
<Compile Include="SQL\TransactionTest\TransactionEnlistmentTest.cs" />
206206
<Compile Include="SQL\UdtTest\SqlServerTypesTest.cs" />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Data;
7+
using System.Runtime.InteropServices;
8+
using System.Threading.Tasks;
9+
using System.Transactions;
10+
using Microsoft.Data.SqlClient.TestUtilities;
11+
using Xunit;
12+
13+
namespace Microsoft.Data.SqlClient.ManualTesting.Tests
14+
{
15+
16+
[PlatformSpecific(TestPlatforms.Windows)]
17+
public class DistributedTransactionTestWindows
18+
{
19+
20+
#if NET
21+
private static bool s_DelegatedTransactionCondition => DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsNotAzureServer() && DataTestUtility.IsNotX86Architecture;
22+
23+
[ConditionalFact(nameof(s_DelegatedTransactionCondition), Timeout = 10000)]
24+
public async Task Delegated_transaction_deadlock_in_SinglePhaseCommit()
25+
{
26+
TransactionManager.ImplicitDistributedTransactions = true;
27+
using var transaction = new CommittableTransaction();
28+
29+
// Uncommenting the following makes the deadlock go away as a workaround. If the transaction is promoted before
30+
// the first SqlClient enlistment, it never goes into the delegated state.
31+
// _ = TransactionInterop.GetTransmitterPropagationToken(transaction);
32+
await using var conn = new SqlConnection(DataTestUtility.TCPConnectionString);
33+
await conn.OpenAsync();
34+
conn.EnlistTransaction(transaction);
35+
36+
// Enlisting the transaction in second connection causes the transaction to be promoted.
37+
// After this, the transaction state will be "delegated" (delegated to SQL Server), and the commit below will
38+
// trigger a call to SqlDelegatedTransaction.SinglePhaseCommit.
39+
await using var conn2 = new SqlConnection(DataTestUtility.TCPConnectionString);
40+
await conn2.OpenAsync();
41+
conn2.EnlistTransaction(transaction);
42+
43+
// Possible deadlock
44+
transaction.Commit();
45+
}
46+
#endif
47+
48+
private static bool s_EnlistedTransactionPreservedWhilePooledCondition => DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsNotX86Architecture;
49+
50+
[ConditionalFact(nameof(s_EnlistedTransactionPreservedWhilePooledCondition), Timeout = 10000)]
51+
public void Test_EnlistedTransactionPreservedWhilePooled()
52+
{
53+
#if NET
54+
TransactionManager.ImplicitDistributedTransactions = true;
55+
#endif
56+
RunTestSet(EnlistedTransactionPreservedWhilePooled);
57+
}
58+
59+
private void EnlistedTransactionPreservedWhilePooled()
60+
{
61+
Exception commandException = null;
62+
Exception transactionException = null;
63+
64+
try
65+
{
66+
using (TransactionScope txScope = new TransactionScope(TransactionScopeOption.Required, TimeSpan.MaxValue))
67+
{
68+
// Leave first connection open so that the transaction is promoted
69+
SqlConnection rootConnection = new SqlConnection(ConnectionString);
70+
rootConnection.Open();
71+
using (SqlCommand command = rootConnection.CreateCommand())
72+
{
73+
command.CommandText = $"INSERT INTO {TestTableName} VALUES ({InputCol1}, '{InputCol2}')";
74+
command.ExecuteNonQuery();
75+
}
76+
77+
// Closing and reopening cycles the connection through the pool.
78+
// We want to verify that the transaction state is preserved through this cycle.
79+
SqlConnection enlistedConnection = new SqlConnection(ConnectionString);
80+
enlistedConnection.Open();
81+
enlistedConnection.Close();
82+
enlistedConnection.Open();
83+
84+
// Forcibly kill the root connection to mimic gateway's behavior when using the proxy connection policy
85+
// https://techcommunity.microsoft.com/blog/azuredbsupport/azure-sql-database-idle-sessions-are-killed-after-about-30-minutes-when-proxy-co/3268601
86+
// Can also represent a general server-side, process failure
87+
KillProcess(rootConnection.ServerProcessId);
88+
89+
90+
using (SqlCommand command = enlistedConnection.CreateCommand())
91+
{
92+
command.CommandText = $"INSERT INTO {TestTableName} VALUES ({InputCol1}, '{InputCol2}')";
93+
try
94+
{
95+
command.ExecuteNonQuery();
96+
}
97+
catch (Exception ex)
98+
{
99+
commandException = ex;
100+
}
101+
}
102+
103+
txScope.Complete();
104+
}
105+
}
106+
catch (Exception ex)
107+
{
108+
transactionException = ex;
109+
}
110+
111+
if (Utils.IsAzureSqlServer(new SqlConnectionStringBuilder((ConnectionString)).DataSource))
112+
{
113+
// Even if an application swallows the command exception, completing the transaction should indicate that it failed.
114+
Assert.IsType<TransactionInDoubtException>(transactionException);
115+
// See https://learn.microsoft.com/en-us/sql/relational-databases/errors-events/database-engine-events-and-errors-3000-to-3999?view=sql-server-ver16
116+
// Error 3971 corresponds to "The server failed to resume the transaction."
117+
Assert.Equal(3971, ((SqlException)commandException).Number);
118+
}
119+
else
120+
{
121+
Assert.IsType<TransactionAbortedException>(transactionException);
122+
123+
#if NETFRAMEWORK
124+
// See https://learn.microsoft.com/en-us/sql/relational-databases/errors-events/database-engine-events-and-errors-8000-to-8999?view=sql-server-ver16
125+
// The distributed transaction failed
126+
Assert.Equal(8525, ((SqlException)commandException).Number);
127+
#else
128+
Assert.IsType<InvalidOperationException>(commandException);
129+
#endif
130+
}
131+
132+
// Verify that nothing made it into the database
133+
DataTable result = DataTestUtility.RunQuery(ConnectionString, $"select col2 from {TestTableName} where col1 = {InputCol1}");
134+
Assert.True(result.Rows.Count == 0);
135+
}
136+
137+
private void KillProcess(int serverProcessId)
138+
{
139+
using (TransactionScope txScope = new TransactionScope(TransactionScopeOption.Suppress))
140+
{
141+
using (SqlConnection connection = new SqlConnection(ConnectionString))
142+
{
143+
connection.Open();
144+
using (SqlCommand command = connection.CreateCommand())
145+
{
146+
command.CommandText = $"KILL {serverProcessId}";
147+
command.ExecuteNonQuery();
148+
}
149+
}
150+
txScope.Complete();
151+
}
152+
}
153+
154+
private static string TestTableName;
155+
private static string ConnectionString;
156+
private const int InputCol1 = 1;
157+
private const string InputCol2 = "One";
158+
159+
private static void RunTestSet(Action TestCase)
160+
{
161+
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString);
162+
163+
builder.Pooling = true;
164+
builder.MaxPoolSize = 5;
165+
builder.Enlist = true;
166+
ConnectionString = builder.ConnectionString;
167+
168+
TestTableName = DataTestUtility.GenerateObjectName();
169+
DataTestUtility.RunNonQuery(ConnectionString, $"create table {TestTableName} (col1 int, col2 text)");
170+
try
171+
{
172+
TestCase();
173+
}
174+
finally
175+
{
176+
DataTestUtility.RunNonQuery(ConnectionString, $"drop table {TestTableName}");
177+
}
178+
}
179+
}
180+
}
181+

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/DistributedTransactionTest.cs

-46
This file was deleted.

0 commit comments

Comments
 (0)