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 3, 2024
1 parent af1047e commit bc4f507
Showing 1 changed file with 50 additions and 22 deletions.
72 changes: 50 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,22 @@ 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 (string, ParameterValueInfo)[] _parameterValues;

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

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

if (_parameterValues.Count > 0)
if (_parameterValues.Length != commandCacheKey._parameterValues.Length)
{
foreach (var (key, value) in _parameterValues)
{
if (!commandCacheKey._parameterValues.TryGetValue(key, out var otherValue))
{
return false;
}
Check.DebugFail("Parameter Count mismatch between identical expressions");
return false;
}

// ReSharper disable once ArrangeRedundantParentheses
if ((value == null) != (otherValue == null))
{
return false;
}
for (var i = 0; i < _parameterValues.Length; i++)
{
var (thisKey, thisValue) = _parameterValues[i];
var (otherKey, otherValue) = commandCacheKey._parameterValues[i];

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 (thisKey != otherKey)
{
Check.DebugFail("Parameter Name mismatch between identical expressions");
return false;
}

if (thisValue.Equals(otherValue))
{
return false;
}
}

Expand All @@ -154,4 +163,23 @@ 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 : IEquatable<ParameterValueInfo>
{
private readonly bool _isNull;

private readonly int? _objectArrayLength;

internal ParameterValueInfo(object? parameterValue) {
_isNull = parameterValue == null;
_objectArrayLength = parameterValue is object[] arr ? arr.Length : null;
}

public bool Equals(ParameterValueInfo other) => _isNull == other._isNull && _objectArrayLength == other._objectArrayLength;

public override bool Equals(object? obj) => obj is ParameterValueInfo other && Equals(other);

public override int GetHashCode() => HashCode.Combine(_isNull, _objectArrayLength);
};
}

0 comments on commit bc4f507

Please sign in to comment.