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

Fix PostEvictionCallbacks bug #46807

Merged
merged 4 commits into from
Jan 11, 2021
Merged

Fix PostEvictionCallbacks bug #46807

merged 4 commits into from
Jan 11, 2021

Conversation

adamsitnik
Copy link
Member

Explanation: after #45962 the this keyword was not referring to an instance of the CacheEntry type:

IDisposable registration = expirationToken.RegisterChangeCallback(ExpirationCallback, this);

And it was causing a silent error after performing an invalid cast as a part of the scheduled task:

Task.Factory.StartNew(state =>
{
var entry = (CacheEntry)state;

Fixes #46774

@ghost
Copy link

ghost commented Jan 11, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Explanation: after #45962 the this keyword was not referring to an instance of the CacheEntry type:

IDisposable registration = expirationToken.RegisterChangeCallback(ExpirationCallback, this);

And it was causing a silent error after performing an invalid cast as a part of the scheduled task:

Task.Factory.StartNew(state =>
{
var entry = (CacheEntry)state;

Fixes #46774

Author: adamsitnik
Assignees: -
Labels:

area-Extensions-Caching

Milestone: 6.0.0

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for getting the fix up so quick.

I just had a couple minor test suggestions.

- remove the dependency to TaskTimeoutExtensions
- don't wait 1s, call Cancel in explicit way
Copy link
Contributor

@pranavkm pranavkm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix!

@adamsitnik adamsitnik merged commit 5282582 into dotnet:master Jan 11, 2021
@adamsitnik adamsitnik deleted the bug46774 branch January 11, 2021 20:06
@ghost ghost locked as resolved and limited conversation to collaborators Feb 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PostEvictionCallbacks does not get invoked when MemoryCache entries expire with an active change token
4 participants