Skip to content

Commit

Permalink
Keep parameter values out IMemoryCache in RelationalCommandCache
Browse files Browse the repository at this point in the history
Store only nullness and array lengths in struct form to prevent parameters memory leaks

Fixes dotnet#34028

Co-authored-by: Shay Rojansky <[email protected]>

(cherry picked from commit af420cd)
  • Loading branch information
yinzara authored and roji committed Oct 14, 2024
1 parent f9b5a38 commit f477f7e
Showing 1 changed file with 23 additions and 20 deletions.
43 changes: 23 additions & 20 deletions src/EFCore.Relational/Query/Internal/RelationalCommandCache.cs
Original file line number Diff line number Diff line change
@@ -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 Microsoft.Extensions.Caching.Memory;

Expand Down Expand Up @@ -106,12 +105,21 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
private readonly struct CommandCacheKey : IEquatable<CommandCacheKey>
{
private readonly Expression _queryExpression;
private readonly IReadOnlyDictionary<string, object?> _parameterValues;
private readonly Dictionary<string, ParameterInfo> _parameterInfos;

public CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues)
internal CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues)
{
_queryExpression = queryExpression;
_parameterValues = parameterValues;
_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)
Expand All @@ -126,27 +134,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;
}
}
}

Expand All @@ -156,4 +155,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);
}

0 comments on commit f477f7e

Please sign in to comment.