Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.1.2] Backport: Release connection lock before signaling SinglePhaseCommit completion #1912

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
RuntimeHelpers.PrepareConstrainedRegions();
try
{
Exception commitException = null;

lock (connection)
{
// If the connection is doomed, we can be certain that the
Expand All @@ -354,7 +356,6 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
}
else
{
Exception commitException;
try
{
// Now that we've acquired the lock, make sure we still have valid state for this operation.
Expand All @@ -364,7 +365,6 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
_connection = null; // Set prior to ExecuteTransaction call in case this initiates a TransactionEnd event

connection.ExecuteTransaction(SqlInternalConnection.TransactionRequest.Commit, null, System.Data.IsolationLevel.Unspecified, _internalTransaction, true);
commitException = null;
}
catch (SqlException e)
{
Expand Down Expand Up @@ -412,13 +412,14 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
}

connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
if (commitException == null)
{
// connection.ExecuteTransaction succeeded
enlistment.Committed();
}
}
}

if (commitException == null)
{
// connection.ExecuteTransaction succeeded
enlistment.Committed();
}
}
catch (System.OutOfMemoryException e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@
<Compile Include="SQL\RandomStressTest\RandomStressTest.cs" />
<Compile Include="SQL\SplitPacketTest\SplitPacketTest.cs" />
<Compile Include="SQL\SqlNotificationTest\SqlNotificationTest.cs" />
<Compile Include="SQL\TransactionTest\DistributedTransactionTest.cs" />
<Compile Include="SQL\TransactionTest\TransactionTest.cs" />
<Compile Include="SQL\TransactionTest\TransactionEnlistmentTest.cs" />
<Compile Include="SQL\UdtTest\SqlServerTypesTest.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Threading.Tasks;
using System.Transactions;
using Xunit;

#if NET7_0_OR_GREATER

namespace Microsoft.Data.SqlClient.ManualTesting.Tests
{
[PlatformSpecific(TestPlatforms.Windows)]
public class DistributedTransactionTest
{
private static bool s_DelegatedTransactionCondition => DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsNotAzureServer() && PlatformDetection.IsNotX86Process;

[ConditionalFact(nameof(s_DelegatedTransactionCondition), Timeout = 10000)]
public async Task Delegated_transaction_deadlock_in_SinglePhaseCommit()
{
TransactionManager.ImplicitDistributedTransactions = true;
using var transaction = new CommittableTransaction();

// Uncommenting the following makes the deadlock go away as a workaround. If the transaction is promoted before
// the first SqlClient enlistment, it never goes into the delegated state.
// _ = TransactionInterop.GetTransmitterPropagationToken(transaction);
await using var conn = new SqlConnection(DataTestUtility.TCPConnectionString);
await conn.OpenAsync();
conn.EnlistTransaction(transaction);

// Enlisting the transaction in second connection causes the transaction to be promoted.
// After this, the transaction state will be "delegated" (delegated to SQL Server), and the commit below will
// trigger a call to SqlDelegatedTransaction.SinglePhaseCommit.
await using var conn2 = new SqlConnection(DataTestUtility.TCPConnectionString);
await conn2.OpenAsync();
conn2.EnlistTransaction(transaction);

// Possible deadlock
transaction.Commit();
}
}
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@ public static partial class PlatformDetection
{
public static bool IsArmProcess => RuntimeInformation.ProcessArchitecture == Architecture.Arm;
public static bool IsNotArmProcess => !IsArmProcess;
public static bool IsX86Process => RuntimeInformation.ProcessArchitecture == Architecture.X86;
public static bool IsNotX86Process => !IsX86Process;
}
}