[release/2.1] Memory Leak in Microsoft.Extensions.Caching.Memory when handling exceptions #3536
Conversation
|
|
Ask mode❔ |
Yep, going to send one email w/ all 3 PRs once they're all green & approved (as well as adding the label) |
|
@Tratcher compiler doesn't like the shorter
|
src/Caching/Memory/src/CacheEntry.cs
Outdated
|
|
||
| internal DateTimeOffset LastAccessed { get; set; } | ||
|
|
||
| internal EvictionReason EvictionReason { get; private set; } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| if (!ValueHasBeenSet) |
There was a problem hiding this comment.
Should _scope be Disposed in this case? It's intiialized in the ctor?
|
@eerhardt @Tratcher |
looking. |
| @@ -13,6 +13,7 @@ | |||
| </PropertyGroup> | |||
| <PropertyGroup Condition=" '$(VersionPrefix)' == '2.1.23' "> | |||
| <PackagesInPatch> | |||
| Microsoft.Extensions.Caching.Memory; | |||
There was a problem hiding this comment.
We are editing the Caching.Abstractions library as well. Do we need to add it to this list?
On my machine, the test is failing because |
Ahhh, that'd do it. Will try fixing up PatchConfig |
|
You also probably need to add a direct reference from the test project to the Abstractions project. 2.1 doesn't lift transitive references. |
|
There has been some changes in the original PR, please make sure this PR is updated with those changes. |
Believe I now have everything except the shorter syntax for |
|
This PR does not have dotnet/runtime@72d5741 yet. |
I just brought it in |
… handling exceptions (dotnet/extensions#3536) * Memory Leak in Microsoft.Extensions.Caching.Memory when handling exceptions * Update patchConfig.props * Try standardizing use of SetValue * Apply feedback * Fix syntax * Undo breaking change * Feedback * Fixup * Fixup patchConfig * Add direct ref * Fix another memory leak when one cache depends on another cache * Fixup\n\nCommit migrated from dotnet/extensions@dc1dab7
ghost
locked as resolved and limited conversation to collaborators
Resolves #3533
Backport of dotnet/runtime#42355
When an exception is thrown inside MemoryCache.GetOrCreate, we are leaking CacheEntry objects. This is because they are not being Disposed properly, and the async local CacheEntryStack is growing indefinitely.
The fix is to ensure the CacheEntry objects are disposed correctly. In order to do this, I set a flag to indicate whether the CacheEntry.Value has been set. If it hasn't, Disposing the CacheEntry won't add it to the cache.