Recover connection state counting after external close

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

@@ -1106,6 +1106,45 @@ public virtual void EnlistTransaction_throws_if_ambient_transaction_started()
             }
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public virtual async Task Externally_closed_connections_are_handled_correctly(bool async)
+        {
+            DbConnection connection;
+            using (var context = CreateContextWithConnectionString())
+            {
+                var set = context.Set<TransactionCustomer>();
+
+                if (async)
+                {
+                    await context.Database.OpenConnectionAsync();
+                }
+                else
+                {
+                    context.Database.OpenConnection();
+                }
+
+                connection = context.Database.GetDbConnection();
+
+                connection.Close();
+
+                var _ = async ? await set.ToListAsync() : set.ToList();
+
+                Assert.Equal(ConnectionState.Open, connection.State);
+
+                context.Database.CloseConnection();
+
+                Assert.Equal(ConnectionState.Closed, connection.State);
+
+                _ = async ? await set.ToListAsync() : set.ToList();
+
+                Assert.Equal(ConnectionState.Closed, connection.State);
+            }
+
+            Assert.Equal(ConnectionState.Closed, connection.State);
+        }
+
         protected virtual void AssertStoreInitialState()
         {
             using (var context = CreateContext())

src/EFCore.Relational/Infrastructure/EntityFrameworkRelationalServicesBuilder.cs

@@ -77,6 +77,7 @@ public static readonly IDictionary<Type, ServiceCharacteristics> RelationalServi
                 { typeof(IMemberTranslator), new ServiceCharacteristics(ServiceLifetime.Singleton) },
                 { typeof(ICompositeMethodCallTranslator), new ServiceCharacteristics(ServiceLifetime.Singleton) },
                 { typeof(IQuerySqlGeneratorFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
+                { typeof(IRelationalTransactionFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
                 { typeof(ICommandBatchPreparer), new ServiceCharacteristics(ServiceLifetime.Scoped) },
                 { typeof(IModificationCommandBatchFactory), new ServiceCharacteristics(ServiceLifetime.Scoped) },
                 { typeof(IMigrationsModelDiffer), new ServiceCharacteristics(ServiceLifetime.Scoped) },
@@ -162,6 +163,7 @@ public override EntityFrameworkServicesBuilder TryAddCoreServices()
             TryAdd<ISqlTranslatingExpressionVisitorFactory, SqlTranslatingExpressionVisitorFactory>();
             TryAdd<INamedConnectionStringResolver, NamedConnectionStringResolver>();
             TryAdd<IEvaluatableExpressionFilter, RelationalEvaluatableExpressionFilter>();
+            TryAdd<IRelationalTransactionFactory, RelationalTransactionFactory>();
 
             ServiceCollectionMap.GetInfrastructure()
                 .AddDependencySingleton<RelationalCompositeMemberTranslatorDependencies>()
@@ -179,6 +181,7 @@ public override EntityFrameworkServicesBuilder TryAddCoreServices()
                 .AddDependencySingleton<SelectExpressionDependencies>()
                 .AddDependencySingleton<RelationalValueBufferFactoryDependencies>()
                 .AddDependencySingleton<RelationalProjectionExpressionVisitorDependencies>()
+                .AddDependencySingleton<RelationalTransactionFactoryDependencies>()
                 .AddDependencyScoped<RelationalConventionSetBuilderDependencies>()
                 .AddDependencyScoped<RelationalDatabaseCreatorDependencies>()
                 .AddDependencyScoped<HistoryRepositoryDependencies>()

src/EFCore.Relational/Query/Internal/AsyncQueryingEnumerable.cs

@@ -100,18 +100,29 @@ private async Task<bool> BufferlessMoveNext(bool buffer, CancellationToken cance
                 {
                     await _relationalQueryContext.Connection.OpenAsync(cancellationToken);
 
-                    var relationalCommand
-                        = _shaperCommandContext
-                            .GetRelationalCommand(_relationalQueryContext.ParameterValues);
-
-                    await _relationalQueryContext.Connection
-                        .RegisterBufferableAsync(this, cancellationToken);
-
-                    _dataReader
-                        = await relationalCommand.ExecuteReaderAsync(
-                            _relationalQueryContext.Connection,
-                            _relationalQueryContext.ParameterValues,
-                            cancellationToken);
+                    try
+                    {
+                        var relationalCommand
+                            = _shaperCommandContext
+                                .GetRelationalCommand(_relationalQueryContext.ParameterValues);
+
+                        await _relationalQueryContext.Connection
+                            .RegisterBufferableAsync(this, cancellationToken);
+
+                        _dataReader
+                            = await relationalCommand.ExecuteReaderAsync(
+                                _relationalQueryContext.Connection,
+                                _relationalQueryContext.ParameterValues,
+                                cancellationToken);
+                    }
+                    catch
+                    {
+                        // If failure happens creating the data reader, then it won't be available to
+                        // handle closing the connection, so do it explicitly here to preserve ref counting.
+                        _relationalQueryContext.Connection.Close();
+
+                        throw;
+                    }
 
                     _dbDataReader = _dataReader.DbDataReader;
                     _shaperCommandContext.NotifyReaderCreated(_dbDataReader);

src/EFCore.Relational/Query/Internal/QueryingEnumerable.cs

@@ -91,16 +91,27 @@ private bool BufferlessMoveNext(bool buffer)
                 {
                     _relationalQueryContext.Connection.Open();
 
-                    var relationalCommand
-                        = _shaperCommandContext
-                            .GetRelationalCommand(_relationalQueryContext.ParameterValues);
+                    try
+                    {
+                        var relationalCommand
+                            = _shaperCommandContext
+                                .GetRelationalCommand(_relationalQueryContext.ParameterValues);
 
-                    _relationalQueryContext.Connection.RegisterBufferable(this);
+                        _relationalQueryContext.Connection.RegisterBufferable(this);
 
-                    _dataReader
-                        = relationalCommand.ExecuteReader(
-                            _relationalQueryContext.Connection,
-                            _relationalQueryContext.ParameterValues);
+                        _dataReader
+                            = relationalCommand.ExecuteReader(
+                                _relationalQueryContext.Connection,
+                                _relationalQueryContext.ParameterValues);
+                    }
+                    catch
+                    {
+                        // If failure happens creating the data reader, then it won't be available to
+                        // handle closing the connection, so do it explicitly here to preserve ref counting.
+                        _relationalQueryContext.Connection.Close();
+
+                        throw;
+                    }
 
                     _dbDataReader = _dataReader.DbDataReader;
                     _shaperCommandContext.NotifyReaderCreated(_dbDataReader);

src/EFCore.Relational/Storage/IRelationalTransactionFactory.cs

@@ -0,0 +1,36 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Data.Common;
+using JetBrains.Annotations;
+using Microsoft.EntityFrameworkCore.Diagnostics;
+
+namespace Microsoft.EntityFrameworkCore.Storage
+{
+    /// <summary>
+    ///     <para>
+    ///         A factory for creating <see cref="RelationalTransaction" /> instances.
+    ///     </para>
+    ///     <para>
+    ///         This type is typically used by database providers It is generally not used in application code.
+    ///     </para>
+    /// </summary>
+    public interface IRelationalTransactionFactory
+    {
+        /// <summary>
+        ///     Creates a <see cref="RelationalTransaction" /> instance.
+        /// </summary>
+        /// <param name="connection"> The connection to the database. </param>
+        /// <param name="transaction"> The underlying <see cref="DbTransaction" />. </param>
+        /// <param name="logger"> The logger to write to. </param>
+        /// <param name="transactionOwned">
+        ///     A value indicating whether the transaction is owned by this class (i.e. if it can be disposed when this class is disposed).
+        /// </param>
+        /// <returns> A new <see cref="RelationalTransaction" /> instance. </returns>
+        RelationalTransaction Create(
+            [NotNull] IRelationalConnection connection,
+            [NotNull] DbTransaction transaction,
+            [NotNull] IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> logger,
+            bool transactionOwned);
+    }
+}

src/EFCore.Relational/Storage/RelationalConnection.cs

@@ -241,7 +241,7 @@ private IDbContextTransaction BeginTransactionWithNoPreconditions(IsolationLevel
             var dbTransaction = DbConnection.BeginTransaction(isolationLevel);
 
             CurrentTransaction
-                = new RelationalTransaction(
+                = Dependencies.RelationalTransactionFactory.Create(
                     this,
                     dbTransaction,
                     Dependencies.TransactionLogger,
@@ -275,7 +275,7 @@ public virtual IDbContextTransaction UseTransaction(DbTransaction transaction)
 
                 Open();
 
-                CurrentTransaction = new RelationalTransaction(
+                CurrentTransaction = Dependencies.RelationalTransactionFactory.Create(
                     this,
                     transaction,
                     Dependencies.TransactionLogger,
@@ -339,10 +339,8 @@ public virtual bool Open(bool errorsExpected = false)
                 wasOpened = true;
                 ClearTransactions();
             }
-            else
-            {
-                _openedCount++;
-            }
+
+            _openedCount++;
 
             HandleAmbientTransactions();
 
@@ -375,10 +373,8 @@ public virtual async Task<bool> OpenAsync(CancellationToken cancellationToken, b
                 wasOpened = true;
                 ClearTransactions();
             }
-            else
-            {
-                _openedCount++;
-            }
+
+            _openedCount++;
 
             HandleAmbientTransactions();
 
@@ -388,7 +384,7 @@ public virtual async Task<bool> OpenAsync(CancellationToken cancellationToken, b
         private void ClearTransactions()
         {
             var previousOpenedCount = _openedCount;
-            _openedCount++;
+            _openedCount += 2;
             CurrentTransaction?.Dispose();
             CurrentTransaction = null;
             EnlistedTransaction = null;
@@ -436,7 +432,6 @@ private void OpenDbConnection(bool errorsExpected)
             if (_openedCount == 0)
             {
                 _openedInternally = true;
-                _openedCount++;
             }
         }
 
@@ -476,7 +471,6 @@ private async Task OpenDbConnectionAsync(bool errorsExpected, CancellationToken
             if (_openedCount == 0)
             {
                 _openedInternally = true;
-                _openedCount++;
             }
         }
 

src/EFCore.Relational/Storage/RelationalConnectionDependencies.cs

@@ -45,21 +45,25 @@ public sealed class RelationalConnectionDependencies
         /// <param name="transactionLogger"> The logger to which transaction messages will be written. </param>
         /// <param name="connectionLogger"> The logger to which connection messages will be written. </param>
         /// <param name="connectionStringResolver"> A service for resolving a connection string from a name. </param>
+        /// <param name="relationalTransactionFactory"> A service for creating <see cref="RelationalTransaction"/> instances. </param>
         public RelationalConnectionDependencies(
             [NotNull] IDbContextOptions contextOptions,
             [NotNull] IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> transactionLogger,
             [NotNull] IDiagnosticsLogger<DbLoggerCategory.Database.Connection> connectionLogger,
-            [NotNull] INamedConnectionStringResolver connectionStringResolver)
+            [NotNull] INamedConnectionStringResolver connectionStringResolver,
+            [NotNull] IRelationalTransactionFactory relationalTransactionFactory)
         {
             Check.NotNull(contextOptions, nameof(contextOptions));
             Check.NotNull(transactionLogger, nameof(transactionLogger));
             Check.NotNull(connectionLogger, nameof(connectionLogger));
             Check.NotNull(connectionStringResolver, nameof(connectionStringResolver));
+            Check.NotNull(relationalTransactionFactory, nameof(relationalTransactionFactory));
 
             ContextOptions = contextOptions;
             TransactionLogger = transactionLogger;
             ConnectionLogger = connectionLogger;
             ConnectionStringResolver = connectionStringResolver;
+            RelationalTransactionFactory = relationalTransactionFactory;
         }
 
         /// <summary>
@@ -82,6 +86,11 @@ public RelationalConnectionDependencies(
         /// </summary>
         public INamedConnectionStringResolver ConnectionStringResolver { get; }
 
+        /// <summary>
+        ///     A service for creating <see cref="RelationalTransaction"/> instances.
+        /// </summary>
+        public IRelationalTransactionFactory RelationalTransactionFactory { get; }
+
         /// <summary>
         ///     Clones this dependency parameter object with one service replaced.
         /// </summary>
@@ -90,7 +99,7 @@ public RelationalConnectionDependencies(
         /// </param>
         /// <returns> A new parameter object with the given service replaced. </returns>
         public RelationalConnectionDependencies With([NotNull] IDbContextOptions contextOptions)
-            => new RelationalConnectionDependencies(contextOptions, TransactionLogger, ConnectionLogger, ConnectionStringResolver);
+            => new RelationalConnectionDependencies(contextOptions, TransactionLogger, ConnectionLogger, ConnectionStringResolver, RelationalTransactionFactory);
 
         /// <summary>
         ///     Clones this dependency parameter object with one service replaced.
@@ -100,7 +109,7 @@ public RelationalConnectionDependencies With([NotNull] IDbContextOptions context
         /// </param>
         /// <returns> A new parameter object with the given service replaced. </returns>
         public RelationalConnectionDependencies With([NotNull] IDiagnosticsLogger<DbLoggerCategory.Database.Connection> connectionLogger)
-            => new RelationalConnectionDependencies(ContextOptions, TransactionLogger, connectionLogger, ConnectionStringResolver);
+            => new RelationalConnectionDependencies(ContextOptions, TransactionLogger, connectionLogger, ConnectionStringResolver, RelationalTransactionFactory);
 
         /// <summary>
         ///     Clones this dependency parameter object with one service replaced.
@@ -110,7 +119,7 @@ public RelationalConnectionDependencies With([NotNull] IDiagnosticsLogger<DbLogg
         /// </param>
         /// <returns> A new parameter object with the given service replaced. </returns>
         public RelationalConnectionDependencies With([NotNull] IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> transactionLogger)
-            => new RelationalConnectionDependencies(ContextOptions, transactionLogger, ConnectionLogger, ConnectionStringResolver);
+            => new RelationalConnectionDependencies(ContextOptions, transactionLogger, ConnectionLogger, ConnectionStringResolver, RelationalTransactionFactory);
 
         /// <summary>
         ///     Clones this dependency parameter object with one service replaced.
@@ -120,6 +129,17 @@ public RelationalConnectionDependencies With([NotNull] IDiagnosticsLogger<DbLogg
         /// </param>
         /// <returns> A new parameter object with the given service replaced. </returns>
         public RelationalConnectionDependencies With([NotNull] INamedConnectionStringResolver connectionStringResolver)
-            => new RelationalConnectionDependencies(ContextOptions, TransactionLogger, ConnectionLogger, connectionStringResolver);
+            => new RelationalConnectionDependencies(ContextOptions, TransactionLogger, ConnectionLogger, connectionStringResolver, RelationalTransactionFactory);
+
+        /// <summary>
+        ///     Clones this dependency parameter object with one service replaced.
+        /// </summary>
+        /// <param name="relationalTransactionFactory">
+        ///     A replacement for the current dependency of this type.
+        /// </param>
+        /// <returns> A new parameter object with the given service replaced. </returns>
+        public RelationalConnectionDependencies With([NotNull] IRelationalTransactionFactory relationalTransactionFactory)
+            => new RelationalConnectionDependencies(ContextOptions, TransactionLogger, ConnectionLogger, ConnectionStringResolver, relationalTransactionFactory);
+
     }
 }

src/EFCore.Relational/Storage/RelationalTransactionFactory.cs

@@ -0,0 +1,54 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Data.Common;
+using JetBrains.Annotations;
+using Microsoft.EntityFrameworkCore.Diagnostics;
+using Microsoft.EntityFrameworkCore.Utilities;
+
+namespace Microsoft.EntityFrameworkCore.Storage
+{
+    /// <summary>
+    ///     <para>
+    ///         A factory for creating <see cref="RelationalTransaction" /> instances.
+    ///     </para>
+    ///     <para>
+    ///         This type is typically used by database providers It is generally not used in application code.
+    ///     </para>
+    /// </summary>
+    public class RelationalTransactionFactory : IRelationalTransactionFactory
+    {
+        /// <summary>
+        ///     Initializes a new instance of the <see cref="RelationalTransactionFactory" /> class.
+        /// </summary>
+        /// <param name="dependencies"> Parameter object containing dependencies for this service. </param>
+        public RelationalTransactionFactory([NotNull] RelationalTransactionFactoryDependencies dependencies)
+        {
+            Check.NotNull(dependencies, nameof(dependencies));
+
+            Dependencies = dependencies;
+        }
+
+        /// <summary>
+        ///     Parameter object containing dependencies for this service.
+        /// </summary>
+        protected virtual RelationalTransactionFactoryDependencies Dependencies { get; }
+
+        /// <summary>
+        ///     Creates a <see cref="RelationalTransaction" /> instance.
+        /// </summary>
+        /// <param name="connection"> The connection to the database. </param>
+        /// <param name="transaction"> The underlying <see cref="DbTransaction" />. </param>
+        /// <param name="logger"> The logger to write to. </param>
+        /// <param name="transactionOwned">
+        ///     A value indicating whether the transaction is owned by this class (i.e. if it can be disposed when this class is disposed).
+        /// </param>
+        /// <returns> A new <see cref="RelationalTransaction" /> instance. </returns>
+        public virtual RelationalTransaction Create(
+            IRelationalConnection connection,
+            DbTransaction transaction,
+            IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> logger,
+            bool transactionOwned)
+            => new RelationalTransaction(connection, transaction, logger, transactionOwned);
+    }
+}