Skip to content

Commit

Permalink
Keep parameter values out of 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 #34028
  • Loading branch information
yinzara committed Oct 4, 2024
1 parent af1047e commit 9a83fa9
Showing 1 changed file with 48 additions and 22 deletions.
70 changes: 48 additions & 22 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,21 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
}
}

private readonly struct CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues)
private readonly struct CommandCacheKey
: IEquatable<CommandCacheKey>
{
private readonly Expression _queryExpression = queryExpression;
private readonly IReadOnlyDictionary<string, object?> _parameterValues = parameterValues;
private readonly Expression _queryExpression;
private readonly ParameterValueInfo[] _parameterValues;

internal CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues) {
_queryExpression = queryExpression;
_parameterValues = new ParameterValueInfo[parameterValues.Count];
var i = 0;
foreach (var (key, value) in parameterValues)
{
_parameterValues[i++] = new ParameterValueInfo(key, value);
}
}

public override bool Equals(object? obj)
=> obj is CommandCacheKey commandCacheKey
Expand All @@ -124,27 +133,27 @@ public bool Equals(CommandCacheKey commandCacheKey)
return false;
}

if (_parameterValues.Count > 0)
Check.DebugAssert(
_parameterValues.Length == commandCacheKey._parameterValues.Length,
"Parameter Count mismatch between identical expressions");

for (var i = 0; i < _parameterValues.Length; i++)
{
foreach (var (key, value) in _parameterValues)
{
if (!commandCacheKey._parameterValues.TryGetValue(key, out var otherValue))
{
return false;
}
var thisValue = _parameterValues[i];
var otherValue = commandCacheKey._parameterValues[i];

// ReSharper disable once ArrangeRedundantParentheses
if ((value == null) != (otherValue == null))
{
return false;
}
Check.DebugAssert(
thisValue.Key == otherValue.Key,
"Parameter Name mismatch between identical expressions");

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 (thisValue.IsNull != otherValue.IsNull)
{
return false;
}

if (thisValue.ObjectArrayLength != otherValue.ObjectArrayLength)
{
return false;
}
}

Expand All @@ -154,4 +163,21 @@ public bool Equals(CommandCacheKey commandCacheKey)
public override int GetHashCode()
=> RuntimeHelpers.GetHashCode(_queryExpression);
}

// Note that we keep only the nullness of parameters (and array length for FromSql object arrays), and avoid referencing the actual parameter data (see #34028).
private readonly struct ParameterValueInfo
{
public string Key { get; }

public bool IsNull { get; }

public int? ObjectArrayLength { get; }

internal ParameterValueInfo(string key, object? parameterValue)
{
Key = key;
IsNull = parameterValue == null;
ObjectArrayLength = parameterValue is object[] arr ? arr.Length : null;
}
}
}

0 comments on commit 9a83fa9

Please sign in to comment.