Skip to content

Fix race condition in EvictionCount_NotOvercounted_WhenEntryAlreadyRemoved test#126826

Merged
cincuranet merged 2 commits intodotnet:mainfrom
cincuranet:fix-memorycache-expiration-race
Apr 13, 2026
Merged

Fix race condition in EvictionCount_NotOvercounted_WhenEntryAlreadyRemoved test#126826
cincuranet merged 2 commits intodotnet:mainfrom
cincuranet:fix-memorycache-expiration-race

Conversation

@cincuranet
Copy link
Copy Markdown
Contributor

Use TimeSpan.MaxValue for ExpirationScanFrequency to prevent background ScanForExpiredItems tasks from racing with the test's explicit TryGetValue and Compact operations. The race caused under-counting because a background scan could remove an entry from the ConcurrentDictionary but not yet increment _accumulatedEvictions when GetCurrentStatistics was called.

Fixes #126818

…nd ScanForExpiredItems tasks from racing with the test's explicit TryGetValue and Compact operations. The race caused under-counting because a background scan could remove an entry from the ConcurrentDictionary but not yet increment _accumulatedEvictions when GetCurrentStatistics was called.

Fixes dotnet#126818
Copilot AI review requested due to automatic review settings April 13, 2026 07:50
@cincuranet cincuranet self-assigned this Apr 13, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-caching
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR stabilizes MemoryCacheMetricsTests.EvictionCount_NotOvercounted_WhenEntryAlreadyRemoved by preventing MemoryCache’s background expiration scanning from racing with the test’s explicit expiration-triggering operations, which previously caused intermittent undercounting of eviction statistics.

Changes:

  • Set MemoryCacheOptions.ExpirationScanFrequency to TimeSpan.MaxValue in the affected test to effectively disable background ScanForExpiredItems scheduling during the test.

Copy link
Copy Markdown
Member

@mrek-msft mrek-msft left a comment

Choose a reason for hiding this comment

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

LGTM. There are TimeSpan.Zero in other tests, but these seems correct to me.

@svick
Copy link
Copy Markdown
Member

svick commented Apr 13, 2026

It's not directly related to this PR, but would it make sense to support Timeout.InfiniteTimeSpan in MemoryCacheOptions.ExpirationScanFrequency? (Though I think it would technically be a breaking change.)

@cincuranet
Copy link
Copy Markdown
Contributor Author

@svick I don't think so. The Timeout.InfiniteTimeSpan is related more to System.Threading. And if you do TimeSpan.MaxValue for ExpirationScanFrequency, it is effectively never.

@cincuranet cincuranet merged commit 6ff5be5 into dotnet:main Apr 13, 2026
94 checks passed
@cincuranet cincuranet deleted the fix-memorycache-expiration-race branch April 13, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failure: Microsoft.Extensions.Caching.Memory.MemoryCacheMetricsTests.EvictionCount_NotOvercounted_WhenEntryAlreadyRemoved

4 participants