dotnet / dotnet/runtime

`MemoryCache` size accounting can be corrupted when `SizeLimit` changes between `null` and a value

Open
#132,610 2 comments 0 reactions 0 assignees View on GitHub
area-Extensions-Caching
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

`MemoryCache` retains the supplied `MemoryCacheOptions` instance and reads `SizeLimit` during every cache operation.

`SizeLimit = null` currently controls two separate behaviours:

1. Whether capacity is enforced.
2. Whether entry sizes are added to and subtracted from `_cacheSize`.

If `SizeLimit` crosses between `null` and a value while an entry remains in the cache, insertion and removal can make different accounting decisions. This permanently corrupts `_cacheSize`.

This is related to #88733, but does not require mutating `ICacheEntry.Size`.

### Reproduction Steps

```csharp
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.Options;

var options = new MemoryCacheOptions
{
TrackStatistics = true
};

using var cache = new MemoryCache(Options.Create(options));

// Size accounting is disabled, so this entry is not added to _cacheSize.
cache.Set("a", "value", new MemoryCacheEntryOptions { Size = 100 });

// Enable size accounting while the entry is still present.
options.SizeLimit = 50;

// Removal now subtracts a size which was never added.
cache.Remove("a");

Console.WriteLine(
cache.GetCurrentStatistics()!.CurrentEstimatedSize); // -100

// The negative value is cast to ulong by the admission check,
// so even a size-1 entry is rejected.
cache.Set("b", "value", new MemoryCacheEntryOptions { Size = 1 });

Console.WriteLine(cache.TryGetValue("b", out _)); // False
```

The reverse transition also corrupts the ledger:

1. Insert an entry while a limit is configured, adding its size.
2. Set `SizeLimit` to `null`.
3. Remove the entry, skipping the subtraction.
4. Re-enable a limit.

The cache is empty, but `_cacheSize` still contains the removed entry's size.

Changing between two non-null limits does not cause this mismatch because size accounting remains enabled throughout.

### Expected behavior

Changing the supplied options object after cache construction must not cause insertion and removal to account for the same entry differently.

### Actual behavior

Insertion and removal independently inspect the current value of `MemoryCacheOptions.SizeLimit`.

This can produce:

- A negative `_cacheSize`, causing every subsequent insertion to be rejected by the unsigned overflow check.
- A positive phantom `_cacheSize`, causing an empty cache to appear full.
- Incorrect replacement and rollback calculations.

Normal insertion, removal and compaction cannot reliably repair the total. `Clear()` or replacing the cache resets it.

### Proposed fix

Separate lifetime size tracking from the current capacity threshold.

`MemoryCache` should capture whether size tracking is enabled when it is constructed:

```csharp
private readonly bool _trackSize;

// Constructor
_trackSize = _options.HasSizeLimit;
```

Use `_trackSize` consistently for:

- Requiring entries to specify `Size`.
- Incrementing `_cacheSize`.
- Replacement accounting.
- Failed-insertion rollback.
- Removal accounting.
- Reporting `CurrentEstimatedSize`.

The numeric `SizeLimitValue` can remain live for admission and compaction.

This permits safe changes between configured limits, for example `50` to `100`, while preserving a stable accounting mode for the cache's lifetime.

If a cache was constructed with `SizeLimit = null`, assigning a limit later would not enable tracking for that existing cache. Supporting that transition fully would require accounting for every existing entry under synchronisation, including defining behaviour for entries without a size.

Contributor guide

Open the contributing guide

Research direction

Start by locating MemoryCache and tracing its insertion, replacement, rollback, removal, compaction, and CurrentEstimatedSize paths, especially where SizeLimit is read. Reproduce both null-to-value and value-to-null transitions from the issue, then verify that accounting remains consistent while capacity admission still follows the current limit.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
66/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.