Clear reference to DbDataReader from RelationalDataReader (dotnet#28989)

Fixes dotnet#28988
roji · Sep 7, 2022 · 4bb67a7 · 4bb67a7
1 parent d453d3f
commit 4bb67a7
Show file tree

Hide file tree

Showing 2 changed files with 102 additions and 10 deletions.
diff --git a/src/EFCore.Relational/Storage/RelationalDataReader.cs b/src/EFCore.Relational/Storage/RelationalDataReader.cs
@@ -173,12 +173,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;
             }
         }
     }
@@ -259,12 +269,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
@@ -0,0 +1,72 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.EntityFrameworkCore.Storage.Internal;
+using Microsoft.EntityFrameworkCore.TestUtilities.FakeProvider;
+
+// 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<string, object>(),
+            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<IRelationalParameter> parameters = null)
+        => new RelationalCommand(
+            new RelationalCommandBuilderDependencies(
+                new TestRelationalTypeMappingSource(
+                    TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
+                    TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>()),
+                new ExceptionDetector()),
+            commandText,
+            parameters ?? Array.Empty<IRelationalParameter>());
+
+    public static IEnumerable<object[]> IsAsyncData = new[] { new object[] { false }, new object[] { true } };
+}