Skip to content

Remove only the expired cache entry TryGet observed, and saturate the TTL [patch] - #62

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/47-cache-expired-remove-race
Sep 29, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/47-cache-expired-remove-race

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #47

What changed

  • Race in TryGet. When TryGet saw an expired entry, it removed whatever was stored under the key at that moment. If another thread called Set in between, the new value was deleted. It now removes the entry by key and value, so it only removes the entry it read:

    • TryRemove(KeyValuePair) on .NET 5+
    • ICollection<KeyValuePair>.Remove on netstandard2.1

    Both compare the value, and CacheEntry is compared by reference, so a replacement set by another thread stays in place.

  • Overflow in Set. Set(key, value, TimeSpan.MaxValue) used to throw ArgumentOutOfRangeException. A new ComputeExpiration saturates instead. A time-to-live that reaches past DateTime.MaxValue never expires. One that reaches before DateTime.MinValue counts as already expired.

Tests

  • Cache_Expired_TryGet_Does_Not_Remove_A_Value_Set_Concurrently makes the race deterministic instead of relying on a stress loop. TryGet hashes the key twice: once to read the entry and once to remove it. The test uses a key type that runs Set(key, 2) on the second hash, which puts the other thread's Set exactly in that window. The test then checks that the fresh value survives.
  • Cache_Set_With_MaxValue_Expiration_Never_Expires and Cache_Set_With_MinValue_Expiration_Has_Already_Expired cover the saturation.

Verification

  • With InMemoryCacheProvider.cs reverted, all three new tests fail. The race test finds the value it just set is gone, and the MaxValue/MinValue tests throw ArgumentOutOfRangeException.
  • With the fix, the full suite passes: 922 of 922 on net10.0.
  • The provider builds for every target, including netstandard2.1, with 0 warnings.
  • CLAUDE.md's description of CacheProviderTests.cs now mentions the new tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CzC1o7LFYHYaoWmVy1NLcp


Generated by Claude Code

… TTL

TryGet removed whatever was stored under the key after seeing an expired
entry, so a value another thread set in between was deleted. It now removes
the entry by key and value. Set treated TimeSpan.MaxValue as an overflow and
threw; a time-to-live past DateTime.MaxValue now never expires.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CzC1o7LFYHYaoWmVy1NLcp
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InMemoryCacheProvider.TryGet can delete a fresh value that another thread just Set, when it sees an expired entry for the same key

2 participants