Release lock before signaling SinglePhaseCommit completion

Fixes #1800

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

@@ -348,6 +348,8 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
 #endif
  try
  {
+ Exception commitException = null;
+
  lock (connection)
  {
  // If the connection is doomed, we can be certain that the
@@ -362,7 +364,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.
@@ -372,7 +373,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)
  {
@@ -420,13 +420,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)
  {

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

@@ -0,0 +1,38 @@
+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
+ {
+ [ConditionalFact]
+ 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);
+
+ transaction.Commit();
+ // We never get here because of the deadlock
+ }
+ }
+}
+
+#endif