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 ae85c4b commit 50e3dbd
Showing 1 changed file with 29 additions and 21 deletions.
50 changes: 29 additions & 21 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 System.Runtime.CompilerServices;
using Microsoft.Extensions.Caching.Memory;
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
}
}

Expand All @@ -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);
}

0 comments on commit 50e3dbd

Please sign in to comment.