[release/10.0] Fix MemoryCache negative _cacheSize drift that permanently latches a size-limited cache

[github-actions]

Backport of #129215 to release/10.0

/cc @cincuranet @sablancoleis

Customer Impact

Customer reported - description from @sablancoleis who reported it:

It's severe, and it's a direct customer impact for us. We don't use this cache as a perf/load optimization, we use it to drive the replication logic between two systems. And because the failure is silent and permanent, every Set is dropped with no exception or log, and it never self-recovers, the cache stays dead until we restart the node. We only even caught it after adding the cache-size telemetry. It happens consistently, across most of our clusters globally. It's not a rare race needing a perfect storm, under normal concurrent load with SizeLimit set, clusters drift into the negative state and stay there. The CentralIndia example is typical: every backend went negative and stayed negative until we restarted.

Regression

From #103931 in .NET 9

Testing

Tests added.

Risk

Medium. Concurrency-sensitive accounting code. Stress test added.

[dotnet-policy-service]

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

[svick]

Hi,

the code complete date for 10.0.10 (the July 2026 release) is 24 June. Make sure to merge this PR on that date at the latest (or explicitly let me know that I should merge it), or it won't make it into that release.

As a reminder, if this is a product change, you also need Tactics approval before merging this PR (test-only or infra-only changes don't require Tactics approval).

[cincuranet]

Approved via email. Waiting for customer validation.

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "2b13233fc12848285f003bb067277ec82c9b4c23",
  "last_dispatched_base_ref": "release/10.0",
  "last_dispatched_base_sha": "eca70963946257444a6e4cfbb06f5eec58156370",
  "last_reviewed_commit": "2b13233fc12848285f003bb067277ec82c9b4c23",
  "last_reviewed_base_ref": "release/10.0",
  "last_reviewed_base_sha": "eca70963946257444a6e4cfbb06f5eec58156370",
  "last_recorded_worker_run_id": "29684900070",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "2b13233fc12848285f003bb067277ec82c9b4c23",
      "review_id": 4730670457
    }
  ]
}

[github-actions]

Holistic Review

Motivation: The problem is real and well-documented. Regression #103931 (shipped in 9.0) moved the prior-entry _cacheSize decrement out of the post-swap if (entryAdded) block into the speculative capacity computation in UpdateCacheSizeExceedsCapacity, before the TryUpdate swap. A concurrent RemoveEntry(priorEntry) in that window double-counts the decrement, driving _cacheSize negative; the (ulong) cast in the capacity check then permanently latches the cache into rejecting every Set. This is confirmed as a silent, permanent, customer-impacting data-loss bug.

Approach: Correct and minimal. The fix keeps #103931's prior-aware capacity check (preserving the #36039 replace-at-limit behavior) but commits only +entry.Size to _cacheSize in UpdateCacheSizeExceedsCapacity, and decrements priorEntry.Size exactly once, atomically tied to a successful TryUpdate swap — restoring the pre-#103931 (8.x) ordering. The failure-path rollback is correctly narrowed to -entry.Size only, since a failed TryUpdate means the prior entry was never swapped out and its size must not be touched here. The only remaining transient window over-counts by a positive priorEntry.Size, which is self-correcting and can never latch the cache.

Summary: ✅ LGTM. This is a faithful backport of merged upstream PR #129215 to release/10.0. I diffed the head commit's MemoryCache.cs against the upstream merge commit (98d5787): the four fix hunks in SetEntry and UpdateCacheSizeExceedsCapacity are byte-for-byte identical; the remaining source differences are unrelated metrics/IMeterFactory features that exist only on main and are correctly absent from the servicing branch. The one intentional test adaptation — IsThreadingSupported in place of upstream's IsMultithreadingSupported — is correct, because IsMultithreadingSupported does not exist in release/10.0's PlatformDetection while IsThreadingSupported does and carries equivalent WASM/browser-skip semantics. The new [OuterLoop] stress test is well-targeted (working set far below SizeLimit, asserts no negative drift and no latching) and would fail against the unfixed code. No actionable findings. This change is servicing-approved and appropriately low-risk for a concurrency fix.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 81.8 AIC · ⌖ 10.7 AIC · ⊞ 10K