Skip to content

Commit 0ba4ef8

Browse files
committed
Preserve distributed transactions on pooled connection reset (#3019)
1 parent 4acabba commit 0ba4ef8

File tree

6 files changed

+195
-54
lines changed

6 files changed

+195
-54
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1598,6 +1598,8 @@ internal void PutObjectFromTransactedPool(DbConnectionInternal obj)
15981598
Debug.Assert(null != obj, "null pooledObject?");
15991599
Debug.Assert(obj.EnlistedTransaction == null, "pooledObject is still enlisted?");
16001600

1601+
obj.DeactivateConnection();
1602+
16011603
// called by the transacted connection pool , once it's removed the
16021604
// connection from it's list. We put the connection back in general
16031605
// circulation.

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -937,11 +937,11 @@ private void ResetConnection()
937937

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

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

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,9 +1081,11 @@ private void ResetConnection()
10811081
// distributed transaction - otherwise don't reset!
10821082
if (Is2000)
10831083
{
1084-
// Prepare the parser for the connection reset - the next time a trip
1085-
// to the server is made.
1086-
_parser.PrepareResetConnection(IsTransactionRoot && !IsNonPoolableTransactionRoot);
1084+
// Pooled connections that are enlisted in a transaction must have their transaction
1085+
// preserved when reseting the connection state. Otherwise, future uses of the connection
1086+
// from the pool will execute outside of the transaction, in auto-commit mode.
1087+
// https://github.com/dotnet/SqlClient/issues/2970
1088+
_parser.PrepareResetConnection(EnlistedTransaction is not null && Pool is not null);
10871089
}
10881090
else if (!IsEnlistedInTransaction)
10891091
{

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

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

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

Lines changed: 0 additions & 45 deletions
This file was deleted.

0 commit comments

Comments
 (0)