Ports Fix deadlock in transaction (#1242) to .NET (#2161)

Author: danielmarbach

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

@@ -348,31 +348,34 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
 #endif
                 try
                 {
-                    Exception commitException = null;
-
-                    lock (connection)
+                    // 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)
                     {
-                        // 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)
                         {
                             try
                             {
                                 // Now that we've acquired the lock, make sure we still have valid state for this operation.
                                 ValidateActiveOnConnection(connection);
 
                                 _active = false; // set to inactive first, doesn't matter how the rest completes, this transaction is done.
-                                _connection = null;   // Set prior to ExecuteTransaction call in case this initiates a TransactionEnd event
+                                _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)
                             {
@@ -391,42 +394,41 @@ public void SinglePhaseCommit(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();
+                            }
+                            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);
                             }
 
-                            connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
+                            // 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.
                         }
-                    }
 
-                    if (commitException == null)
-                    {
-                        // connection.ExecuteTransaction succeeded
-                        enlistment.Committed();
+                        connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
+                        if (commitException == null)
+                        {
+                            // connection.ExecuteTransaction succeeded
+                            enlistment.Committed();
+                        }
                     }
                 }
                 catch (System.OutOfMemoryException e)

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

@@ -387,7 +387,6 @@ public void SinglePhaseCommit(SysTx.SinglePhaseEnlistment enlistment)
             Debug.Assert(null != enlistment, "null enlistment?");
             SqlInternalConnection connection = GetValidConnection();
 
-
             if (null != connection)
             {
                 SqlConnection usersConnection = connection.Connection;

src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs

@@ -439,6 +439,17 @@ public static bool IsTargetReadyForAeWithKeyStore()
                 ;
         }
 
+        public static bool IsSupportingDistributedTransactions()
+        {
+#if NET7_0_OR_GREATER
+            return OperatingSystem.IsWindows() && System.Runtime.InteropServices.RuntimeInformation.ProcessArchitecture != System.Runtime.InteropServices.Architecture.X86 && IsNotAzureServer();
+#elif NETFRAMEWORK
+            return IsNotAzureServer();
+#else
+            return false;
+#endif
+        }
+
         public static bool IsUsingManagedSNI() => UseManagedSNIOnWindows;
 
         public static bool IsNotUsingManagedSNIOnWindows() => !UseManagedSNIOnWindows;

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

@@ -47,11 +47,10 @@ public static void TestManualEnlistment_Enlist_TxScopeComplete()
             RunTestSet(TestCase_ManualEnlistment_Enlist_TxScopeComplete);
         }
 
-        [SkipOnTargetFramework(TargetFrameworkMonikers.Netcoreapp)]
-        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsSupportingDistributedTransactions))]
         public static void TestEnlistmentPrepare_TxScopeComplete()
         {
-            try
+            Assert.Throws<TransactionAbortedException>( () =>
             {
                 using TransactionScope txScope = new(TransactionScopeOption.RequiresNew, new TransactionOptions()
                 {
@@ -63,12 +62,7 @@ public static void TestEnlistmentPrepare_TxScopeComplete()
                 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()