Nested ambient transaction

src/EFCore.Relational/Properties/RelationalStrings.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!--
-    Microsoft ResX Schema
-
+  <!-- 
+    Microsoft ResX Schema 
+    
     Version 2.0
-
-    The primary goals of this format is to allow a simple XML format
-    that is mostly human readable. The generation and parsing of the
-    various data types are done through the TypeConverter classes
+    
+    The primary goals of this format is to allow a simple XML format 
+    that is mostly human readable. The generation and parsing of the 
+    various data types are done through the TypeConverter classes 
     associated with the data types.
-
+    
     Example:
-
+    
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-
-    There are any number of "resheader" rows that contain simple
+                
+    There are any number of "resheader" rows that contain simple 
     name/value pairs.
-
-    Each data row contains a name, and value. The row also contains a
-    type or mimetype. Type corresponds to a .NET class that support
-    text/value conversion through the TypeConverter architecture.
-    Classes that don't support this are serialized and stored with the
+    
+    Each data row contains a name, and value. The row also contains a 
+    type or mimetype. Type corresponds to a .NET class that support 
+    text/value conversion through the TypeConverter architecture. 
+    Classes that don't support this are serialized and stored with the 
     mimetype set.
-
-    The mimetype is used for serialized objects, and tells the
-    ResXResourceReader how to depersist the object. This is currently not
+    
+    The mimetype is used for serialized objects, and tells the 
+    ResXResourceReader how to depersist the object. This is currently not 
     extensible. For a given mimetype the value must be set accordingly:
-
-    Note - application/x-microsoft.net.object.binary.base64 is the format
-    that the ResXResourceWriter will generate, however the reader can
+    
+    Note - application/x-microsoft.net.object.binary.base64 is the format 
+    that the ResXResourceWriter will generate, however the reader can 
     read any of the formats listed below.
-
+    
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with
+    value   : The object must be serialized with 
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-
+    
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with
+    value   : The object must be serialized with 
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array
+    value   : The object must be serialized into a byte array 
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -503,4 +503,7 @@
   <data name="FromSqlNonComposable" xml:space="preserve">
     <value>FromSqlRaw or FromSqlInterpolated was called with non-composable SQL and with a query composing over it. Consider calling `AsEnumerable` after the FromSqlRaw or FromSqlInterpolated method to perform the composition on the client side.</value>
   </data>
+  <data name="NestedAmbientTransactionError" xml:space="preserve">
+    <value>Internal ambient transaction is to be completed prior to the root one.</value>
+  </data>
 </root>

src/EFCore.Relational/Storage/RelationalConnection.cs

@@ -2,15 +2,19 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
+using System.Collections.Concurrent;
 using System.Data;
 using System.Data.Common;
 using System.Diagnostics;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using System.Transactions;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Utilities;
 using Microsoft.Extensions.DependencyInjection;
 using IsolationLevel = System.Data.IsolationLevel;
@@ -39,7 +43,7 @@ public abstract class RelationalConnection : IRelationalConnection, ITransaction
         private int _openedCount;
         private bool _openedInternally;
         private int? _commandTimeout;
-        private Transaction _ambientTransaction;
+        private readonly ConcurrentStack<Transaction> _ambientTransactions;
         private DbConnection _connection;
 
         /// <summary>
@@ -76,6 +80,7 @@ protected RelationalConnection([NotNull] RelationalConnectionDependencies depend
             {
                 _connectionOwned = true;
             }
+            _ambientTransactions = new ConcurrentStack<Transaction>();
         }
 
         /// <summary>
@@ -548,11 +553,16 @@ private void ClearTransactions(bool clearAmbient)
         {
             CurrentTransaction = null;
             EnlistedTransaction = null;
-            if (clearAmbient
-                && _ambientTransaction != null)
+            if (clearAmbient && _ambientTransactions.Count > 0)
             {
-                _ambientTransaction.TransactionCompleted -= HandleTransactionCompleted;
-                _ambientTransaction = null;
+                while (_ambientTransactions.Any(t => t != null))
+                {
+                    _ambientTransactions.TryPop(out var ambientTransaction);
+                    if (ambientTransaction != null)
+                    {
+                        ambientTransaction.TransactionCompleted -= HandleTransactionCompleted;
+                    }
+                }
             }
 
             if (_openedCount < 0)
@@ -629,53 +639,52 @@ await Dependencies.ConnectionLogger.ConnectionErrorAsync(
         private void HandleAmbientTransactions()
         {
             var current = Transaction.Current;
-            if (current != null
-                && !SupportsAmbientTransactions)
+
+            if (current == null) 
             {
-                Dependencies.TransactionLogger.AmbientTransactionWarning(this, DateTimeOffset.UtcNow);
+                var rootTransaction = _ambientTransactions.Any() && _ambientTransactions.TryPeek(out var transaction) ? transaction : null;
+
+                if (rootTransaction != null) 
+                {
+                    throw new InvalidOperationException(RelationalStrings.PendingAmbientTransaction);
+                }
+
                 return;
             }
 
-            if (Equals(current, _ambientTransaction))
+            if (!SupportsAmbientTransactions)
             {
+                Dependencies.TransactionLogger.AmbientTransactionWarning(this, DateTimeOffset.UtcNow);
                 return;
             }
 
-            if (_ambientTransaction != null)
+            if (_ambientTransactions.Contains(current))
             {
-                throw new InvalidOperationException(RelationalStrings.PendingAmbientTransaction);
+                return;
             }
 
-            if (current != null)
-            {
-                Dependencies.TransactionLogger.AmbientTransactionEnlisted(this, current);
-            }
+            Dependencies.TransactionLogger.AmbientTransactionEnlisted(this, current);
+            current.TransactionCompleted += HandleTransactionCompleted;
 
             DbConnection.EnlistTransaction(current);
-
-            var ambientTransaction = _ambientTransaction;
-            if (ambientTransaction != null)
-            {
-                ambientTransaction.TransactionCompleted -= HandleTransactionCompleted;
-            }
-
-            if (current != null)
-            {
-                current.TransactionCompleted += HandleTransactionCompleted;
-            }
-
-            _ambientTransaction = current;
+            _ambientTransactions.Push(current);
         }
 
         private void HandleTransactionCompleted(object sender, TransactionEventArgs e)
         {
             // This could be invoked on a different thread at arbitrary time after the transaction completes
-            var ambientTransaction = _ambientTransaction;
+            _ambientTransactions.TryPeek(out var ambientTransaction);
+            if (e.Transaction != ambientTransaction)
+            {
+                throw new InvalidOperationException(RelationalStrings.NestedAmbientTransactionError);
+            }
+
             if (ambientTransaction != null)
             {
                 ambientTransaction.TransactionCompleted -= HandleTransactionCompleted;
-                _ambientTransaction = null;
             }
+
+            _ambientTransactions.TryPop(out var _);
         }
 
         /// <summary>

test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs

@@ -434,6 +434,100 @@ public virtual void SaveChanges_throws_for_suppressed_ambient_transactions(bool
             AssertStoreInitialState();
         }
 
+        [ConditionalFact]
+        public virtual void SaveChanges_allows_nested_ambient_transactions()
+        {
+            if (!AmbientTransactionsSupported)
+            {
+                return;
+            }
+
+            if (TestStore.ConnectionState == ConnectionState.Closed)
+            {
+                TestStore.OpenConnection();
+            }
+
+            using (var context = CreateContext())
+            {
+                using (var tr = new TransactionScope())
+                {
+                    context.Add(new TransactionCustomer { Id = 77, Name = "Bobbie" });
+                    context.SaveChanges();
+                    tr.Complete();
+                    TestStore.CloseConnection();
+                    using (var nestedTransaction = new TransactionScope(TransactionScopeOption.RequiresNew))
+                    {
+                        context.Add(new TransactionOrder { Id = 300, Name = "Order3" });
+                        context.SaveChanges();
+                        nestedTransaction.Complete();
+                        TestStore.CloseConnection();
+                    }
+                }
+                Assert.Equal(
+                    new List<int>
+                    {
+                        1,
+                        2,
+                        77
+                    },
+                    context.Set<TransactionCustomer>().OrderBy(c => c.Id).Select(e => e.Id).ToList());
+                Assert.Equal(
+                    new List<int>
+                    {
+                        100,
+                        200,
+                        300
+                    },
+                    context.Set<TransactionOrder>().OrderBy(c => c.Id).Select(e => e.Id).ToList());
+            }
+        }
+
+        [ConditionalFact]
+        public virtual void SaveChanges_allows_independent_ambient_transaction_commits()
+        {
+            if (!AmbientTransactionsSupported)
+            {
+                return;
+            }
+
+            if (TestStore.ConnectionState == ConnectionState.Closed)
+            {
+                TestStore.OpenConnection();
+            }
+
+            using (var context = CreateContext())
+            {
+                using (var tr = new TransactionScope())
+                {
+                    context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" });
+                    context.SaveChanges();
+                    TestStore.CloseConnection();
+                    using (var nestedTransaction = new TransactionScope(TransactionScopeOption.RequiresNew))
+                    {
+                        context.Add(new TransactionOrder { Id = 300, Name = "Order3" });
+                        context.SaveChanges();
+                        nestedTransaction.Complete();
+                        TestStore.CloseConnection();
+                    }
+                }
+                Assert.Equal(
+                    new List<int>
+                    {
+                        1,
+                        2
+                    },
+                    context.Set<TransactionCustomer>().OrderBy(c => c.Id).Select(e => e.Id).ToList());
+                Assert.Equal(
+                    new List<int>
+                    {
+                        100,
+                        200,
+                        300
+                    },
+                    context.Set<TransactionOrder>().OrderBy(c => c.Id).Select(e => e.Id).ToList());
+            }
+        }
+
         [ConditionalFact]
         public virtual void SaveChanges_uses_enlisted_transaction_after_ambient_transaction()
         {
@@ -1231,11 +1325,19 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         ps.Property(c => c.Id).ValueGeneratedNever();
                         ps.ToTable("Customers");
                     });
+                modelBuilder.Entity<TransactionOrder>(
+                    ps =>
+                    {
+                        ps.Property(c => c.Id).ValueGeneratedNever();
+                        ps.ToTable("Orders");
+                    });
             }
 
             protected override void Seed(PoolableDbContext context)
             {
                 context.AddRange(Customers);
+                context.AddRange(Orders);
+
                 context.SaveChanges();
             }
         }
@@ -1245,11 +1347,15 @@ protected override void Seed(PoolableDbContext context)
             new TransactionCustomer { Id = 1, Name = "Bob" }, new TransactionCustomer { Id = 2, Name = "Dave" }
         };
 
-        protected class TransactionCustomer
+        protected static readonly IReadOnlyList<TransactionOrder> Orders = new List<TransactionOrder>
+        {
+            new TransactionOrder { Id = 100, Name = "Order1" }, new TransactionOrder { Id = 200, Name = "Order2" }
+        };
+
+        protected abstract class TransactionEntity
         {
             public int Id { get; set; }
             public string Name { get; set; }
-
             public override bool Equals(object obj)
             {
                 return !(obj is TransactionCustomer otherCustomer)
@@ -1261,6 +1367,10 @@ public override bool Equals(object obj)
             public override string ToString() => "Id = " + Id + ", Name = " + Name;
 
             public override int GetHashCode() => HashCode.Combine(Id, Name);
+
         }
+        protected class TransactionCustomer : TransactionEntity { }
+
+        protected class TransactionOrder : TransactionEntity { }
     }
 }