From 5379920aa5a0b18c1debec573cd677f360a521fe Mon Sep 17 00:00:00 2001
From: Matthew Vance <yinzara@gmail.com>
Date: Mon, 14 Oct 2024 13:41:32 -0700
Subject: [PATCH] Keep parameter values out IMemoryCache in
 RelationalCommandCache (#34803)

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

Fixes #34028

Co-authored-by: Shay Rojansky <roji@roji.org>
(cherry picked from commit af420cd58113725dcbe36b02348d835cefc898a1)
---
 .../Query/Internal/RelationalCommandCache.cs  | 50 +++++++++++--------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs b/src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs
index b8729a4cedf..c4edba62a40 100644
--- a/src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs
+++ b/src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs
@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections;
 using System.Collections.Concurrent;
 using System.Runtime.CompilerServices;
 using Microsoft.Extensions.Caching.Memory;
@@ -106,11 +105,25 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
         }
     }
 
-    private readonly struct CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues)
-        : IEquatable<CommandCacheKey>
+    private readonly struct CommandCacheKey : IEquatable<CommandCacheKey>
     {
-        private readonly Expression _queryExpression = queryExpression;
-        private readonly IReadOnlyDictionary<string, object?> _parameterValues = parameterValues;
+        private readonly Expression _queryExpression;
+        private readonly Dictionary<string, ParameterInfo> _parameterInfos;
+
+        internal CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues)
+        {
+            _queryExpression = queryExpression;
+            _parameterInfos = new Dictionary<string, ParameterInfo>();
+
+            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)
             => obj is CommandCacheKey commandCacheKey
@@ -124,27 +137,18 @@ 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)
+                foreach (var (key, info) in _parameterInfos)
                 {
-                    if (!commandCacheKey._parameterValues.TryGetValue(key, out var otherValue))
+                    if (!commandCacheKey._parameterInfos.TryGetValue(key, out var otherInfo) || info != otherInfo)
                     {
                         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;
-                    }
                 }
             }
 
@@ -154,4 +158,8 @@ public bool Equals(CommandCacheKey commandCacheKey)
         public override int GetHashCode()
             => RuntimeHelpers.GetHashCode(_queryExpression);
     }
+
+    // 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);
 }