From 38f72ca76f22663fda0b387dffb5356c1ee7f0e4 Mon Sep 17 00:00:00 2001
From: Shay Rojansky <roji@roji.org>
Date: Tue, 15 Oct 2024 01:47:11 +0200
Subject: [PATCH] [release/8.0] Keep parameter values out IMemoryCache in
 RelationalCommandCache (#34908)

Store only nullness and array lengths in struct form to prevent parameters memory leaks

Fixes #34028

Co-authored-by: Matthew Vance <yinzara@gmail.com>
---
 .../Query/Internal/RelationalCommandCache.cs  | 77 +++++++++++++++----
 1 file changed, 60 insertions(+), 17 deletions(-)

diff --git a/src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs b/src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs
index d0c18bb5afd..2eceda4584d 100644
--- a/src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs
+++ b/src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs
@@ -105,13 +105,35 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
 
     private readonly struct CommandCacheKey : IEquatable<CommandCacheKey>
     {
+        private static readonly bool UseOldBehavior34201 =
+            AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue34028", out var enabled34028) && enabled34028;
+
         private readonly Expression _queryExpression;
-        private readonly IReadOnlyDictionary<string, object?> _parameterValues;
+        private readonly Dictionary<string, ParameterInfo> _parameterInfos;
+
+        // Quirk only
+        private readonly IReadOnlyDictionary<string, object?>? _parameterValues;
 
         public CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues)
         {
             _queryExpression = queryExpression;
-            _parameterValues = parameterValues;
+            _parameterInfos = new Dictionary<string, ParameterInfo>();
+
+            if (UseOldBehavior34201)
+            {
+                _parameterValues = parameterValues;
+            }
+            else
+            {
+                foreach (var (key, value) in parameterValues)
+                {
+                    _parameterInfos[key] = new ParameterInfo
+                    {
+                        IsNull = value is null,
+                        ObjectArrayLength = value is object[] arr ? arr.Length : null
+                    };
+                }
+            }
         }
 
         public override bool Equals(object? obj)
@@ -126,26 +148,43 @@ public bool Equals(CommandCacheKey commandCacheKey)
                 return false;
             }
 
-            if (_parameterValues.Count > 0)
+            Check.DebugAssert(
+                _parameterInfos.Count == commandCacheKey._parameterInfos.Count,
+                "Parameter Count mismatch between identical queries");
+
+            if (_parameterInfos.Count > 0)
             {
-                foreach (var (key, value) in _parameterValues)
+                if (UseOldBehavior34201)
                 {
-                    if (!commandCacheKey._parameterValues.TryGetValue(key, out var otherValue))
-                    {
-                        return false;
-                    }
-
-                    // ReSharper disable once ArrangeRedundantParentheses
-                    if ((value == null) != (otherValue == null))
+                    foreach (var (key, value) in _parameterValues!)
                     {
-                        return false;
+                        if (!_parameterValues.TryGetValue(key, out var otherValue))
+                        {
+                            return false;
+                        }
+
+                        // ReSharper disable once ArrangeRedundantParentheses
+                        if ((value == null) != (otherValue == null))
+                        {
+                            return false;
+                        }
+
+                        if (value is IEnumerable
+                            && value.GetType() == typeof(object[]))
+                        {
+                            // FromSql parameters must have the same number of elements
+                            return ((object[])value).Length == (otherValue as object[])?.Length;
+                        }
                     }
-
-                    if (value is IEnumerable
-                        && value.GetType() == typeof(object[]))
+                }
+                else
+                {
+                    foreach (var (key, info) in _parameterInfos)
                     {
-                        // FromSql parameters must have the same number of elements
-                        return ((object[])value).Length == (otherValue as object[])?.Length;
+                        if (!commandCacheKey._parameterInfos.TryGetValue(key, out var otherInfo) || info != otherInfo)
+                        {
+                            return false;
+                        }
                     }
                 }
             }
@@ -156,4 +195,8 @@ public bool Equals(CommandCacheKey commandCacheKey)
         public override int GetHashCode()
             => 0;
     }
+
+    // Note that we keep only the null-ness of parameters (and array length for FromSql object arrays),
+    // and avoid referencing the actual parameter data (see #34028).
+    private readonly record struct ParameterInfo(bool IsNull, int? ObjectArrayLength);
 }