From 15865e629cafe4c4cfa49d33c69041a4e3e1ef88 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Tue, 26 Jul 2022 10:55:32 +0100 Subject: [PATCH] Close the DbConnection when returning a context with a DbConnection to the pool Fixes #27308 --- .../Storage/RelationalConnection.cs | 32 +- .../DbContextPoolingTest.cs | 305 ++++++++++++++++++ 2 files changed, 325 insertions(+), 12 deletions(-) diff --git a/src/EFCore.Relational/Storage/RelationalConnection.cs b/src/EFCore.Relational/Storage/RelationalConnection.cs index e0e1260cb3e..dce6c603715 100644 --- a/src/EFCore.Relational/Storage/RelationalConnection.cs +++ b/src/EFCore.Relational/Storage/RelationalConnection.cs @@ -1036,14 +1036,18 @@ protected virtual void ResetState(bool disposeDbConnection) _openedCount = 0; _openedInternally = false; - if (disposeDbConnection - && _connectionOwned + if (_connectionOwned && _connection is not null) { - DisposeDbConnection(); - _connection = null; - _openedCount = 0; - _openedInternally = false; + CloseDbConnection(); + + if (disposeDbConnection) + { + DisposeDbConnection(); + _connection = null; + _openedCount = 0; + _openedInternally = false; + } } } @@ -1065,14 +1069,18 @@ protected virtual async ValueTask ResetStateAsync(bool disposeDbConnection) _commandTimeout = _defaultCommandTimeout; - if (disposeDbConnection - && _connectionOwned + if (_connectionOwned && _connection is not null) { - await DisposeDbConnectionAsync().ConfigureAwait(false); - _connection = null; - _openedCount = 0; - _openedInternally = false; + await CloseDbConnectionAsync().ConfigureAwait(continueOnCapturedContext: false); + + if (disposeDbConnection) + { + await DisposeDbConnectionAsync().ConfigureAwait(false); + _connection = null; + _openedCount = 0; + _openedInternally = false; + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs b/test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs index adc61fa1e57..6b983f68861 100644 --- a/test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Data; +using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore.Internal; // ReSharper disable MethodHasAsyncOverload @@ -1384,6 +1386,309 @@ public async Task Provider_services_are_reset_with_factory(bool async, bool with Assert.Null(context3.Database.CurrentTransaction); } + [ConditionalTheory] // Issue #27308. + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + public async Task Handle_open_connection_when_returning_to_pool_for_owned_connection(bool async, bool openWithEf) + { + //using var connection = new SqlConnection(SqlServerNorthwindTestStoreFactory.NorthwindConnectionString); + var serviceProvider = new ServiceCollection() + .AddDbContextPool( + ob => ob.UseSqlServer(SqlServerNorthwindTestStoreFactory.NorthwindConnectionString) + .EnableServiceProviderCaching(false)) + .BuildServiceProvider(validateScopes: true); + + var serviceScope = serviceProvider.CreateScope(); + var scopedProvider = serviceScope.ServiceProvider; + + var context1 = scopedProvider.GetRequiredService(); + var connection1 = context1.Database.GetDbConnection(); + + if (async) + { + if (openWithEf) + { + await context1.Database.OpenConnectionAsync(); + } + else + { + await connection1.OpenAsync(); + } + } + else + { + if (openWithEf) + { + context1.Database.OpenConnection(); + } + else + { + connection1.Open(); + } + } + + Assert.Equal(ConnectionState.Open, connection1.State); + + await Dispose(serviceScope, async); + + Assert.Equal(ConnectionState.Closed, connection1.State); + + serviceScope = serviceProvider.CreateScope(); + scopedProvider = serviceScope.ServiceProvider; + + var context2 = scopedProvider.GetRequiredService(); + Assert.Same(context1, context2); + + var connection2 = context2.Database.GetDbConnection(); + Assert.Same(connection1, connection2); + + Assert.Equal(ConnectionState.Closed, connection1.State); + + await Dispose(serviceScope, async); + } + + [ConditionalTheory] // Issue #27308. + [InlineData(false, false, false)] + [InlineData(true, false, false)] + [InlineData(false, true, false)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(true, false, true)] + public async Task Handle_open_connection_when_returning_to_pool_for_external_connection(bool async, bool startsOpen, bool openWithEf) + { + using var connection = new SqlConnection(SqlServerNorthwindTestStoreFactory.NorthwindConnectionString); + + if (startsOpen) + { + if (async) + { + await connection.OpenAsync(); + } + else + { + connection.Open(); + } + } + + var serviceProvider = new ServiceCollection() + .AddDbContextPool( + ob => ob.UseSqlServer(connection) + .EnableServiceProviderCaching(false)) + .BuildServiceProvider(validateScopes: true); + + var serviceScope = serviceProvider.CreateScope(); + var scopedProvider = serviceScope.ServiceProvider; + + var context1 = scopedProvider.GetRequiredService(); + Assert.Same(connection, context1.Database.GetDbConnection()); + + if (!startsOpen) + { + if (async) + { + if (openWithEf) + { + await context1.Database.OpenConnectionAsync(); + } + else + { + await connection.OpenAsync(); + } + } + else + { + if (openWithEf) + { + context1.Database.OpenConnection(); + } + else + { + connection.Open(); + } + } + } + + Assert.Equal(ConnectionState.Open, connection.State); + + await Dispose(serviceScope, async); + + Assert.Equal(ConnectionState.Open, connection.State); + + serviceScope = serviceProvider.CreateScope(); + scopedProvider = serviceScope.ServiceProvider; + + var context2 = scopedProvider.GetRequiredService(); + Assert.Same(context1, context2); + + Assert.Same(connection, context2.Database.GetDbConnection()); + + Assert.Equal(ConnectionState.Open, connection.State); + + await Dispose(serviceScope, async); + } + + [ConditionalTheory] // Issue #27308. + [InlineData(false, false, false)] + [InlineData(true, false, false)] + [InlineData(false, true, false)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(true, false, true)] + [InlineData(false, true, true)] + [InlineData(true, true, true)] + public async Task Handle_open_connection_when_returning_to_pool_for_owned_connection_with_factory( + bool async, bool openWithEf, bool withDependencyInjection) + { + var options = new DbContextOptionsBuilder() + .UseSqlServer(SqlServerNorthwindTestStoreFactory.NorthwindConnectionString) + .EnableServiceProviderCaching(false) + .Options; + + var factory = + withDependencyInjection + ? new ServiceCollection() + .AddPooledDbContextFactory( + ob => ob.UseSqlServer(SqlServerNorthwindTestStoreFactory.NorthwindConnectionString) + .EnableServiceProviderCaching(false)) + .BuildServiceProvider(validateScopes: true) + .GetRequiredService>() + : new PooledDbContextFactory(options); + + var context1 = async ? await factory.CreateDbContextAsync() : factory.CreateDbContext(); + var connection1 = context1.Database.GetDbConnection(); + + if (async) + { + if (openWithEf) + { + await context1.Database.OpenConnectionAsync(); + } + else + { + await connection1.OpenAsync(); + } + } + else + { + if (openWithEf) + { + context1.Database.OpenConnection(); + } + else + { + connection1.Open(); + } + } + + Assert.Equal(ConnectionState.Open, connection1.State); + + await Dispose(context1, async); + + Assert.Equal(ConnectionState.Closed, connection1.State); + + var context2 = async ? await factory.CreateDbContextAsync() : factory.CreateDbContext(); + Assert.Same(context1, context2); + + var connection2 = context2.Database.GetDbConnection(); + Assert.Same(connection1, connection2); + + Assert.Equal(ConnectionState.Closed, connection1.State); + + await Dispose(context2, async); + } + + [ConditionalTheory] // Issue #27308. + [InlineData(false, false, false, false)] + [InlineData(true, false, false, false)] + [InlineData(false, true, false, false)] + [InlineData(true, true, false, false)] + [InlineData(false, false, true, false)] + [InlineData(true, false, true, false)] + [InlineData(false, false, false, true)] + [InlineData(true, false, false, true)] + [InlineData(false, true, false, true)] + [InlineData(true, true, false, true)] + [InlineData(false, false, true, true)] + [InlineData(true, false, true, true)] + public async Task Handle_open_connection_when_returning_to_pool_for_external_connection_with_factory( + bool async, bool startsOpen, bool openWithEf, bool withDependencyInjection) + { + using var connection = new SqlConnection(SqlServerNorthwindTestStoreFactory.NorthwindConnectionString); + + if (startsOpen) + { + if (async) + { + await connection.OpenAsync(); + } + else + { + connection.Open(); + } + } + + var options = new DbContextOptionsBuilder() + .UseSqlServer(connection) + .EnableServiceProviderCaching(false) + .Options; + + var factory = + withDependencyInjection + ? new ServiceCollection() + .AddPooledDbContextFactory( + ob => ob.UseSqlServer(connection) + .EnableServiceProviderCaching(false)) + .BuildServiceProvider(validateScopes: true) + .GetRequiredService>() + : new PooledDbContextFactory(options); + + var context1 = async ? await factory.CreateDbContextAsync() : factory.CreateDbContext(); + Assert.Same(connection, context1.Database.GetDbConnection()); + + if (!startsOpen) + { + if (async) + { + if (openWithEf) + { + await context1.Database.OpenConnectionAsync(); + } + else + { + await connection.OpenAsync(); + } + } + else + { + if (openWithEf) + { + context1.Database.OpenConnection(); + } + else + { + connection.Open(); + } + } + } + + Assert.Equal(ConnectionState.Open, connection.State); + + await Dispose(context1, async); + + Assert.Equal(ConnectionState.Open, connection.State); + + var context2 = async ? await factory.CreateDbContextAsync() : factory.CreateDbContext(); + Assert.Same(context1, context2); + + Assert.Same(connection, context2.Database.GetDbConnection()); + + Assert.Equal(ConnectionState.Open, connection.State); + + await Dispose(context2, async); + } + [ConditionalTheory] [InlineData(true)] [InlineData(false)]