Visitar URL original
Fix HybridCache reservation over-release on DisableUnderlyingData miss by Eng3bbas · Pull Request #7879 · dotnet/extensions · GitHub
Skip to content

Fix HybridCache reservation over-release on DisableUnderlyingData miss - #7879

Merged
svick merged 2 commits into
dotnet:mainfrom
Eng3bbas:hybridcache-default-result-refcount
Nov 2, 2026
Merged

Fix HybridCache reservation over-release on DisableUnderlyingData miss#7879
svick merged 2 commits into
dotnet:mainfrom
Eng3bbas:hybridcache-default-result-refcount

Conversation

@Eng3bbas

@Eng3bbas Eng3bbas commented Oct 23, 2026 •
edited by dotnet-policy-service Bot
Loading

Copy link
Copy Markdown
Contributor

Split out of #7876, as suggested there; this fix doesn't depend on the GetAsync API.

Problem

When GetOrCreateAsync runs with HybridCacheEntryFlags.DisableUnderlyingData and the value is not in L2, StampedeState<TState, T>.SetDefaultResult completed the shared result with ImmutableCacheItem<T>.GetReservedShared(): one item holding a single reservation. Every caller joined to that stampede then calls GetReservedValue, which releases one reservation each. With N joined callers, the item is released N times against 1 reservation:

  • Debug builds hit Debug.Assert(newCount >= 0, "over-release detected") in CacheItem.Release.
  • Release builds silently drive the shared item's reservation counter negative, so it no longer tracks how many callers hold the item.

Fix

SetDefaultResult now completes with the stampede's own CacheItem, which already holds one reservation per joined caller (added by TryAddCaller). The default value is assigned through SetValue / SetFallbackValue, depending on whether the item is immutable or mutable. The item is still not written to L1 or L2. GetReservedShared is no longer used and is removed.

Tests

StampedeTests.JoinedCallersWithoutUnderlyingDataBalanceReservations_{Immutable,Mutable} join 1 or 10 callers to a DisableUnderlyingData stampede, run it, and check that:

  • the completed item holds exactly one reservation per caller;
  • it is back to zero after every caller has unwrapped it.

All four cases fail on main and pass with this change. The full Microsoft.Extensions.Caching.Hybrid.Tests suite passes on net8.0, net9.0, and net10.0.

🤖 Generated with Claude Code

Microsoft Reviewers: Open in CodeFlow

@Eng3bbas @claude
Fix HybridCache reservation over-release on DisableUnderlyingData miss
cb6bc4b 
When a stampede had DisableUnderlyingData set and the value was not
found in L2, every joined caller was completed with one shared
ImmutableCacheItem that held a single reservation. Each caller releases
one reservation when it unwraps the result, so every caller beyond the
first over-released the shared item (a Debug.Assert in debug builds, a
drifting counter in release builds).

The miss now completes with the stampede's own cache item, which
already holds one reservation per joined caller, so the counts balance.
The unused GetReservedShared helper is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 14:32
@Eng3bbas
Eng3bbas requested a review from a team as a code owner October 23, 2026 14:32
Copilot AI reviewed Oct 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@svick svick linked an issue Oct 23, 2026 that may be closed by this pull request
HybridCache over-releases the shared default cache item on concurrent DisableUnderlyingData misses #7880
Closed
@dotnet-comment-bot

dotnet-comment-bot commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

‼️ Found issues ‼️

Project Coverage Type Expected Actual
Microsoft.Extensions.Diagnostics.Testing Line 99 98.65 🔻
Microsoft.Extensions.Telemetry Line 93 92.75 🔻
Microsoft.Extensions.AI Line 89 88.4 🔻
Microsoft.Extensions.AI.OpenAI Line 75 68.24 🔻
Microsoft.Extensions.AI.OpenAI Branch 75 57.54 🔻
Microsoft.Extensions.DataIngestion.MarkItDown Line 75 4.46 🔻
Microsoft.Extensions.DataIngestion.MarkItDown Branch 75 0 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring Line 99 96.07 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring Branch 99 92.76 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring.Kubernetes Line 99 97.73 🔻
Microsoft.Extensions.ServiceDiscovery.Abstractions Line 75 42.11 🔻
Microsoft.Extensions.ServiceDiscovery.Abstractions Branch 75 42.86 🔻
Microsoft.Extensions.ServiceDiscovery.Dns Line 75 69.93 🔻
Microsoft.Extensions.ServiceDiscovery Line 75 68.11 🔻
Microsoft.Extensions.ServiceDiscovery Branch 75 71.43 🔻
Microsoft.Extensions.ServiceDiscovery.Yarp Line 75 73.85 🔻
Microsoft.Extensions.ServiceDiscovery.Yarp Branch 75 70 🔻
Microsoft.Extensions.VectorData.Abstractions Line 75 37.39 🔻
Microsoft.Extensions.VectorData.Abstractions Branch 75 22.73 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project Expected Actual
Microsoft.Extensions.Http.Diagnostics 94 95
Microsoft.Gen.BuildMetadata 97 100
Microsoft.Gen.MetadataExtractor 57 73
Microsoft.Gen.MetricsReports 67 69
Microsoft.Extensions.AI.Abstractions 82 86
Microsoft.Extensions.AI.Evaluation.NLP 0 78
Microsoft.Extensions.Caching.Hybrid 82 85
Microsoft.Extensions.DataIngestion 75 89
Microsoft.Extensions.DataIngestion.Markdig 75 90
Microsoft.Extensions.Http.Resilience 97 100

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1646953&view=codecoverage-tab

@svick
svick requested review from mrek-msft, rosebyte and svick and a balanced review from Copilot October 29, 2026 15:48
Copilot AI reviewed Oct 29, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No review issues remain outstanding for the HybridCache reservation fix.

No open issues 🎉

Review effort: Balanced

svick
svick approved these changes Oct 29, 2026
View reviewed changes

@svick svick left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR. This looks good to me.

Comment thread test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/StampedeTests.cs Resolved
Abbas Hasneen Mohamed Al-Khateeb @claude Abbas Hasneen Mohamed Al-Khateeb
Use Me() for the stampede key in the reservation test
aef8cea 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dotnet-comment-bot

dotnet-comment-bot commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator

‼️ Found issues ‼️

Project Coverage Type Expected Actual
Microsoft.Extensions.Diagnostics.Testing Line 99 98.65 🔻
Microsoft.Extensions.Telemetry Line 93 92.75 🔻
Microsoft.Extensions.AI Line 89 88.4 🔻
Microsoft.Extensions.AI.OpenAI Line 75 68.24 🔻
Microsoft.Extensions.AI.OpenAI Branch 75 57.54 🔻
Microsoft.Extensions.DataIngestion.MarkItDown Line 75 4.46 🔻
Microsoft.Extensions.DataIngestion.MarkItDown Branch 75 0 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring Line 99 96.07 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring Branch 99 92.76 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring.Kubernetes Line 99 97.73 🔻
Microsoft.Extensions.ServiceDiscovery.Dns Line 75 69.93 🔻
Microsoft.Extensions.ServiceDiscovery.Abstractions Line 75 42.11 🔻
Microsoft.Extensions.ServiceDiscovery.Abstractions Branch 75 42.86 🔻
Microsoft.Extensions.ServiceDiscovery Line 75 68.88 🔻
Microsoft.Extensions.ServiceDiscovery Branch 75 71.43 🔻
Microsoft.Extensions.ServiceDiscovery.Yarp Line 75 73.85 🔻
Microsoft.Extensions.ServiceDiscovery.Yarp Branch 75 70 🔻
Microsoft.Extensions.VectorData.Abstractions Line 75 37.39 🔻
Microsoft.Extensions.VectorData.Abstractions Branch 75 22.73 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project Expected Actual
Microsoft.Extensions.Http.Diagnostics 94 95
Microsoft.Gen.BuildMetadata 97 100
Microsoft.Gen.MetadataExtractor 57 73
Microsoft.Gen.MetricsReports 67 69
Microsoft.Extensions.AI.Abstractions 82 86
Microsoft.Extensions.AI.Evaluation.NLP 0 78
Microsoft.Extensions.Caching.Hybrid 82 85
Microsoft.Extensions.DataIngestion 75 89
Microsoft.Extensions.DataIngestion.Markdig 75 90
Microsoft.Extensions.Http.Resilience 97 100

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1654573&view=codecoverage-tab

@svick
svick merged commit 2a41d61 into dotnet:main Nov 2, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@svick svick svick approved these changes
@mrek-msft mrek-msft mrek-msft approved these changes
@rosebyte rosebyte Awaiting requested review from rosebyte

Assignees

@Eng3bbas Eng3bbas

Labels

area-caching-hybrid

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

HybridCache over-releases the shared default cache item on concurrent DisableUnderlyingData misses

5 participants

@Eng3bbas @dotnet-comment-bot @svick @mrek-msft