Skip to content

Commit

Permalink
Keep parameter values out IMemoryCache in RelationalCommandCache (dot…
Browse files Browse the repository at this point in the history
…net#34803)

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 5379920
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 5379920

Please sign in to comment.