[Release 3.0] Fix deadlock in transaction (.NET Framework)

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

@@ -391,21 +391,23 @@ public void SinglePhaseCommit(SysTx.SinglePhaseEnlistment enlistment)
 #else
  {
 #endif //DEBUG
- lock (connection)
+ // If the connection is doomed, we can be certain that the
+ // transaction will eventually be rolled back, and we shouldn't
+ // attempt to commit it.
+ if (connection.IsConnectionDoomed)
  {
- // If the connection is doomed, we can be certain that the
- // transaction will eventually be rolled back or has already been aborted externally, and we shouldn't
- // attempt to commit it.
- if (connection.IsConnectionDoomed)
+ lock (connection)
  {
  _active = false; // set to inactive first, doesn't matter how the rest completes, this transaction is done.
  _connection = null;
-
- enlistment.Aborted(SQL.ConnectionDoomed());
  }
- else
+ enlistment.Aborted(SQL.ConnectionDoomed());
+ }
+ else
+ {
+ Exception commitException;
+ lock (connection)
  {
- Exception commitException;
  try
  {
  // Now that we've acquired the lock, make sure we still have valid state for this operation.
@@ -434,40 +436,40 @@ public void SinglePhaseCommit(SysTx.SinglePhaseEnlistment enlistment)
  ADP.TraceExceptionWithoutRethrow(e);
  connection.DoomThisConnection();
  }
- if (commitException != null)
+ }
+ if (commitException != null)
+ {
+ // connection.ExecuteTransaction failed with exception
+ if (_internalTransaction.IsCommitted)
  {
- // connection.ExecuteTransaction failed with exception
- if (_internalTransaction.IsCommitted)
- {
- // Even though we got an exception, the transaction
- // was committed by the server.
- enlistment.Committed();
- }
- else if (_internalTransaction.IsAborted)
- {
- // The transaction was aborted, report that to
- // SysTx.
- enlistment.Aborted(commitException);
- }
- else
- {
- // The transaction is still active, we cannot
- // know the state of the transaction.
- enlistment.InDoubt(commitException);
- }
-
- // We eat the exception. This is called on the SysTx
- // thread, not the applications thread. If we don't 
- // eat the exception an UnhandledException will occur,
- // causing the process to FailFast.
+ // Even though we got an exception, the transaction
+ // was committed by the server.
+ enlistment.Committed();
  }
-
- connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
- if (commitException == null)
+ else if (_internalTransaction.IsAborted)
  {
- // connection.ExecuteTransaction succeeded
- enlistment.Committed();
+ // The transaction was aborted, report that to
+ // SysTx.
+ enlistment.Aborted(commitException);
  }
+ else
+ {
+ // The transaction is still active, we cannot
+ // know the state of the transaction.
+ enlistment.InDoubt(commitException);
+ }
+
+ // We eat the exception. This is called on the SysTx
+ // thread, not the applications thread. If we don't 
+ // eat the exception an UnhandledException will occur,
+ // causing the process to FailFast.
+ }
+
+ connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
+ if (commitException == null)
+ {
+ // connection.ExecuteTransaction succeeded
+ enlistment.Committed();
  }
  }
  }

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

@@ -4,6 +4,7 @@
 
 using System;
 using System.Data;
+using System.Threading.Tasks;
 using System.Transactions;
 using Xunit;
 
@@ -46,6 +47,30 @@ public static void TestManualEnlistment_Enlist_TxScopeComplete()
  RunTestSet(TestCase_ManualEnlistment_Enlist_TxScopeComplete);
  }
 
+ [SkipOnTargetFramework(TargetFrameworkMonikers.Netcoreapp)]
+ [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
+ public static void TestEnlistmentPrepare_TxScopeComplete()
+ {
+ try
+ {
+ using TransactionScope txScope = new(TransactionScopeOption.RequiresNew, new TransactionOptions()
+ {
+ IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted,
+ Timeout = TransactionManager.DefaultTimeout
+ }, TransactionScopeAsyncFlowOption.Enabled);
+
+ using SqlConnection connection = new(DataTestUtility.TCPConnectionString);
+ connection.Open();
+ System.Transactions.Transaction.Current.EnlistDurable(EnlistmentForPrepare.s_id, new EnlistmentForPrepare(), EnlistmentOptions.None);
+ txScope.Complete();
+ Assert.False(true, "Expected exception not thrown.");
+ }
+ catch (Exception e)
+ {
+ Assert.True(e is TransactionAbortedException);
+ }
+ }
+
  private static void TestCase_AutoEnlistment_TxScopeComplete()
  {
  SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(ConnectionString);
@@ -168,6 +193,16 @@ private static void TestCase_ManualEnlistment_Enlist_TxScopeComplete()
  Assert.True(string.Equals(result.Rows[0][0], InputCol2));
  }
 
+ class EnlistmentForPrepare : IEnlistmentNotification
+ {
+ public static readonly Guid s_id = Guid.NewGuid();
+ // fail during prepare, this will cause scope.Complete to throw
+ public void Prepare(PreparingEnlistment preparingEnlistment) => preparingEnlistment.ForceRollback();
+ public void Commit(Enlistment enlistment) => enlistment.Done();
+ public void Rollback(Enlistment enlistment) => enlistment.Done();
+ public void InDoubt(Enlistment enlistment) => enlistment.Done();
+ }
+
  private static string TestTableName;
  private static string ConnectionString;
  private const int InputCol1 = 1;