Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/3.1] Memory Leak in Microsoft.Extensions.Caching.Memory when handling exceptions #3535

Merged
merged 4 commits into from
Sep 18, 2020

Conversation

wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Sep 17, 2020

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.

@Tratcher
Copy link
Member

Note dotnet/runtime#42355 still has feedback that needs to be applied.

@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 17, 2020

Note dotnet/runtime#42355 still has feedback that needs to be applied.

@Tratcher the compiler doesn't like the shorter using syntax here, might not be on a new enough version of C#

/home/vsts/work/1/s/src/Caching/Abstractions/src/MemoryCacheExtensions.cs(38,19): error CS1003: Syntax error, '(' expected [/home/vsts/work/1/s/src/Caching/Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj]

Will apply the rest

@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 17, 2020

Feedback has been applied

@JunTaoLuo
Copy link

There has been some changes in the original PR, please make sure this PR is updated with those changes.

@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 17, 2020

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 using, which the compiler in 3.1/2.1 doesn't know about

@wtgodbe wtgodbe merged commit 5dc3e89 into release/3.1 Sep 18, 2020
@wtgodbe wtgodbe deleted the wtgodbe/MemLeak branch September 18, 2020 18:54
JunTaoLuo pushed a commit to dotnet/aspnetcore that referenced this pull request Nov 17, 2020
… handling exceptions (dotnet/extensions#3535)

* Memory Leak in Microsoft.Extensions.Caching.Memory when handling exceptions

* Fixup

* Fixup

* Fix another memory leak when one cache depends on another cache\n\nCommit migrated from dotnet/extensions@5dc3e89
@ghost ghost locked as resolved and limited conversation to collaborators May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants