From c5950aad0994637c3db936605091de38dd022149 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 7 Sep 2022 09:43:44 +0300 Subject: [PATCH] Clear reference to DbDataReader from RelationalDataReader (#28989) Fixes #28988 (cherry picked from commit 4bb67a7b56d10f7989f0115b1d978be418018dc8) --- .../Storage/RelationalDataReader.cs | 40 +++++++--- .../Storage/RelationalDataReaderTest.cs | 76 +++++++++++++++++++ 2 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 test/EFCore.Relational.Tests/Storage/RelationalDataReaderTest.cs diff --git a/src/EFCore.Relational/Storage/RelationalDataReader.cs b/src/EFCore.Relational/Storage/RelationalDataReader.cs index f4ae8f34535..abbed1796ef 100644 --- a/src/EFCore.Relational/Storage/RelationalDataReader.cs +++ b/src/EFCore.Relational/Storage/RelationalDataReader.cs @@ -131,12 +131,22 @@ public virtual void Dispose() { _disposed = true; - if (!interceptionResult.IsSuppressed) + try { - _reader.Dispose(); - _command.Parameters.Clear(); - _command.Dispose(); - _relationalConnection.Close(); + if (!interceptionResult.IsSuppressed) + { + _reader.Dispose(); + _command.Parameters.Clear(); + _command.Dispose(); + _relationalConnection.Close(); + } + } + finally + { + _reader = null!; + _command = null!; + _relationalConnection = null!; + _logger = null; } } } @@ -171,12 +181,22 @@ public virtual async ValueTask DisposeAsync() { _disposed = true; - if (!interceptionResult.IsSuppressed) + try + { + if (!interceptionResult.IsSuppressed) + { + await _reader.DisposeAsync().ConfigureAwait(false); + _command.Parameters.Clear(); + await _command.DisposeAsync().ConfigureAwait(false); + await _relationalConnection.CloseAsync().ConfigureAwait(false); + } + } + finally { - await _reader.DisposeAsync().ConfigureAwait(false); - _command.Parameters.Clear(); - await _command.DisposeAsync().ConfigureAwait(false); - await _relationalConnection.CloseAsync().ConfigureAwait(false); + _reader = null!; + _command = null!; + _relationalConnection = null!; + _logger = null; } } } diff --git a/test/EFCore.Relational.Tests/Storage/RelationalDataReaderTest.cs b/test/EFCore.Relational.Tests/Storage/RelationalDataReaderTest.cs new file mode 100644 index 00000000000..7f07e75b046 --- /dev/null +++ b/test/EFCore.Relational.Tests/Storage/RelationalDataReaderTest.cs @@ -0,0 +1,76 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Microsoft.EntityFrameworkCore.TestUtilities.FakeProvider; +using Xunit; + +// ReSharper disable MethodHasAsyncOverload + +namespace Microsoft.EntityFrameworkCore.Storage; + +public class RelationalDataReaderTest +{ + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public async Task Does_not_hold_reference_to_DbDataReader_after_dispose(bool async) + { + var fakeConnection = CreateConnection(); + var relationalCommand = CreateRelationalCommand(commandText: "CommandText"); + + var reader = relationalCommand.ExecuteReader(new( + fakeConnection, + new Dictionary(), + readerColumns: null, + context: null, + logger: null)); + + Assert.NotNull(reader.DbDataReader); + + if (async) + { + await reader.DisposeAsync(); + } + else + { + reader.Dispose(); + } + + Assert.Null(reader.DbDataReader); + } + + private const string ConnectionString = "Fake Connection String"; + + private static FakeRelationalConnection CreateConnection(IDbContextOptions options = null) + => new(options ?? CreateOptions()); + + private static IDbContextOptions CreateOptions( + RelationalOptionsExtension optionsExtension = null) + { + var optionsBuilder = new DbContextOptionsBuilder(); + + ((IDbContextOptionsBuilderInfrastructure)optionsBuilder) + .AddOrUpdateExtension( + optionsExtension + ?? new FakeRelationalOptionsExtension().WithConnectionString(ConnectionString)); + + return optionsBuilder.Options; + } + + private IRelationalCommand CreateRelationalCommand( + string commandText = "Command Text", + IReadOnlyList parameters = null) + => new RelationalCommand( + new RelationalCommandBuilderDependencies( + new TestRelationalTypeMappingSource( + TestServiceFactory.Instance.Create(), + TestServiceFactory.Instance.Create())), + commandText, + parameters ?? Array.Empty()); + + public static IEnumerable IsAsyncData = new[] { new object[] { false }, new object[] { true } }; +}