[release/8.0] Keep parameter values out IMemoryCache in RelationalCommandCache #34908
Conversation
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)
|
Consider adding a quirk mode |
Done, PTAL. Should we include the quirk in 9.0 as well, given that GA is right around the corner? Feels like if we want to potentially give people a way out of this change for 8.0, the same should hold for 9.0 at this point. |
9.0 is not LTS, so I think it's just a bit more risk-tolerant. |
| private readonly Dictionary<string, ParameterInfo> _parameterInfos; | ||
|
|
||
| // Quirk only | ||
| private readonly IReadOnlyDictionary<string, object?>? _parameterValues; | ||
|
|
||
| public CommandCacheKey(Expression queryExpression, IReadOnlyDictionary<string, object?> parameterValues) |
There was a problem hiding this comment.
Man, still getting this leak. Primary constructor still holding it probably
There was a problem hiding this comment.
@bleapleaper can you please open a new issue with a minimal, runnable repro that shows this still happening after the fix?
There was a problem hiding this comment.
Unable to repro with the repro in the issue, but in another project i still see the leak. Investigating further. Too trigger happy on the comments sorry
Store only nullness and array lengths in struct form to prevent parameters memory leaks
Fixes #34028
Backports #34028
Description
EF's query pipeline contains a SQL cache, whose key contains the nullness of parameters; this is needed since different nullness requires different SQL (e.g.
WHERE b.Name = @p(where p is non-null), vs.WHERE b.Name IS NULL(where p is null)). The cache key implementation currently references the parameter value, rather than just recording whether it was null or not. This causes user data to be referenced from internal caching, keeping potentially large amounts of data referenced and preventing the GC from reclaiming them.Customer impact
In some cases, large amounts of user data will be kept in memory and will never get garbage-collected.
How found
Customer reported on 8.0
Regression
No
Testing
Testing that a reference is kept to an object isn't feasible in a reliable way.
Risk
Low.