From e5a39680aa216c86b124eeb7ff99462cececa105 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Thu, 23 Apr 2026 16:06:09 +0200 Subject: [PATCH 01/16] Implement HybridCacheFactoryContext support in DefaultHybridCache Override GetOrCreateAsync with context-aware factory in DefaultHybridCache. The implementation: - Adds context-aware QueueUserWorkItem/ExecuteDirectAsync overloads to StampedeState that create and pass HybridCacheFactoryContext - Invokes the context-aware factory in BackgroundFetchAsync - Applies write-side flags (DisableLocalCacheWrite, DisableDistributedCacheWrite, DisableCompression) from the context after factory execution; read-side flags are ignored - Composes new HybridCacheEntryOptions from context expiration overrides - Applies LocalCacheSize override to both ImmutableCacheItem and MutableCacheItem - Adds MutableCacheItem.SetLocalCacheSizeOverride for mutable type size control - Handles the invalid-key path with a context-aware RunWithoutCacheAsync overload - Centralizes effective post-factory flags in SetResult(activeFlags) to avoid checking the immutable Key.Flags for factory-overridden write decisions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DefaultHybridCache.MutableCacheItem.cs | 8 +- .../DefaultHybridCache.StampedeStateT.cs | 114 +++++++++++++++--- .../Internal/DefaultHybridCache.cs | 77 ++++++++++++ 3 files changed, 183 insertions(+), 16 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs index c48e01969bf..7e71d81b196 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs @@ -13,6 +13,7 @@ internal sealed partial class MutableCacheItem : CacheItem // used to hold private IHybridCacheSerializer? _serializer; private BufferChunk _buffer; private T? _fallbackValue; // only used in the case of serialization failures + private long _localCacheSizeOverride = -1; // -1 means use buffer length public MutableCacheItem(long creationTimestamp, TagSet tags) : base(creationTimestamp, tags) @@ -66,7 +67,7 @@ public override bool TryGetSize(out long size) // only if we haven't already burned if (TryReserve()) { - size = _buffer.Length; + size = _localCacheSizeOverride >= 0 ? _localCacheSizeOverride : _buffer.Length; _ = Release(); return true; } @@ -75,6 +76,11 @@ public override bool TryGetSize(out long size) return false; } + public void SetLocalCacheSizeOverride(long size) + { + _localCacheSizeOverride = size; + } + public override bool TryReserveBuffer(out BufferChunk buffer) { // only if we haven't already burned diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index 34a68ef30aa..8107b6a58c1 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -23,6 +23,8 @@ internal sealed class StampedeState : StampedeState private readonly TaskCompletionSource>? _result; private TState? _state; private Func>? _underlying; // main data factory + private Func>? _underlyingWithContext; // context-aware data factory + private HybridCacheFactoryContext? _factoryContext; private HybridCacheEntryOptions? _options; private Task? _sharedUnwrap; // allows multiple non-cancellable callers to share a single task (when no defensive copy needed) @@ -75,6 +77,39 @@ public Task ExecuteDirectAsync(in TState state, Func> underlying, HybridCacheEntryOptions? options) + { + Debug.Assert(_underlyingWithContext is null, "should not already have factory field"); + Debug.Assert(underlying is not null, "factory argument should be meaningful"); + + // initialize the callback state + _state = state; + _underlyingWithContext = underlying; + _factoryContext = new HybridCacheFactoryContext(); + _options = options; + +#if NETCOREAPP3_0_OR_GREATER + ThreadPool.UnsafeQueueUserWorkItem(this, false); +#else + ThreadPool.UnsafeQueueUserWorkItem(SharedWaitCallback, this); +#endif + } + + [SuppressMessage("Resilience", "EA0014:The async method doesn't support cancellation", Justification = "Cancellation is handled separately via SharedToken")] + public Task ExecuteDirectAsync(in TState state, Func> underlying, HybridCacheEntryOptions? options) + { + Debug.Assert(_underlyingWithContext is null, "should not already have factory field"); + Debug.Assert(underlying is not null, "factory argument should be meaningful"); + + // initialize the callback state + _state = state; + _underlyingWithContext = underlying; + _factoryContext = new HybridCacheFactoryContext(); + _options = options; + + return BackgroundFetchAsync(); + } + public override void Execute() => _ = BackgroundFetchAsync(); public override void SetCanceled() => _result?.TrySetCanceled(SharedToken); @@ -265,7 +300,14 @@ private async Task BackgroundFetchAsync() HybridCacheEventSource.Log.UnderlyingDataQueryStart(); } - newValue = await _underlying!(_state!, SharedToken).ConfigureAwait(false); + if (_underlyingWithContext is not null) + { + newValue = await _underlyingWithContext(_state!, _factoryContext!, SharedToken).ConfigureAwait(false); + } + else + { + newValue = await _underlying!(_state!, SharedToken).ConfigureAwait(false); + } if (eventSourceEnabled) { @@ -289,6 +331,12 @@ private async Task BackgroundFetchAsync() throw; } + // apply context overrides set by the factory callback + if (_factoryContext is not null) + { + ApplyFactoryContext(_factoryContext, ref activeFlags, ref _options); + } + // check whether we're going to hit a timing problem with tag invalidation if (!Cache.IsValid(CacheItem)) { @@ -326,12 +374,14 @@ private async Task BackgroundFetchAsync() // Rephrasing that: the only scenario in which we *do not* need to serialize is if: // - it is an ImmutableCacheItem (so we don't need bytes for the CacheItem, L1) // - we're not writing to L2 + // Additionally, if the caller provided an explicit LocalCacheSize via the factory context, + // we can also skip serialization for size purposes (we already know the size). CacheItem cacheItem = CacheItem; bool skipSerialize = cacheItem is ImmutableCacheItem && (activeFlags & FlagsDisableL1AndL2Write) == FlagsDisableL1AndL2Write; if (skipSerialize) { - SetImmutableResultWithoutSerialize(newValue); + SetImmutableResultWithoutSerialize(newValue, activeFlags); } else if (cacheItem.TryReserve()) { @@ -354,7 +404,7 @@ private async Task BackgroundFetchAsync() buffer = buffer.DoNotReturnToPool(); // set the underlying result for this operation (includes L1 write if appropriate) - SetResultPreSerialized(newValue, ref bufferToRelease, serializer); + SetResultPreSerialized(newValue, ref bufferToRelease, serializer, activeFlags); // Note that at this point we've already released most or all of the waiting callers. Everything // from this point onwards happens in the background, from the perspective of the calling code. @@ -385,7 +435,7 @@ private async Task BackgroundFetchAsync() { // unable to serialize (or quota exceeded); try to at least store the onwards value; this is // especially useful for immutable data types - SetResultPreSerialized(newValue, ref bufferToRelease, serializer); + SetResultPreSerialized(newValue, ref bufferToRelease, serializer, activeFlags); } // Release our hook on the CacheItem (only really important for "mutable"). @@ -461,12 +511,12 @@ private void SetResultAndRecycleIfAppropriate(ref BufferChunk value, TimeSpan re break; } - SetResult(cacheItem, remainingTime); + SetResult(cacheItem, Key.Flags, remainingTime); } - private void SetImmutableResultWithoutSerialize(T value) + private void SetImmutableResultWithoutSerialize(T value, HybridCacheEntryFlags activeFlags) { - Debug.Assert((Key.Flags & FlagsDisableL1AndL2Write) == FlagsDisableL1AndL2Write, "Only expected if L1+L2 disabled"); + Debug.Assert((activeFlags & FlagsDisableL1AndL2Write) == FlagsDisableL1AndL2Write, "Only expected if L1+L2 disabled"); // set a result from a value we calculated directly CacheItem cacheItem; @@ -474,7 +524,7 @@ private void SetImmutableResultWithoutSerialize(T value) { case ImmutableCacheItem immutable: // no serialize needed - immutable.SetValue(value, size: -1); + immutable.SetValue(value, size: _factoryContext?.LocalCacheSize ?? -1); cacheItem = immutable; break; default: @@ -482,10 +532,10 @@ private void SetImmutableResultWithoutSerialize(T value) break; } - SetResult(cacheItem); + SetResult(cacheItem, activeFlags); } - private void SetResultPreSerialized(T value, ref BufferChunk buffer, IHybridCacheSerializer? serializer) + private void SetResultPreSerialized(T value, ref BufferChunk buffer, IHybridCacheSerializer? serializer, HybridCacheEntryFlags activeFlags) { // set a result from a value we calculated directly that // has ALREADY BEEN SERIALIZED (we can optionally consume this buffer) @@ -494,7 +544,7 @@ private void SetResultPreSerialized(T value, ref BufferChunk buffer, IHybridCach { case ImmutableCacheItem immutable: // no serialize needed - immutable.SetValue(value, size: buffer.Length); + immutable.SetValue(value, size: _factoryContext?.LocalCacheSize ?? buffer.Length); cacheItem = immutable; // (but leave the buffer alone) @@ -511,6 +561,11 @@ private void SetResultPreSerialized(T value, ref BufferChunk buffer, IHybridCach mutable.DebugOnlyTrackBuffer(Cache); } + if (_factoryContext?.LocalCacheSize is { } mutableSize) + { + mutable.SetLocalCacheSizeOverride(mutableSize); + } + cacheItem = mutable; break; default: @@ -518,14 +573,14 @@ private void SetResultPreSerialized(T value, ref BufferChunk buffer, IHybridCach break; } - SetResult(cacheItem); + SetResult(cacheItem, activeFlags); } - private void SetResult(CacheItem value) => SetResult(value, TimeSpan.MaxValue); + private void SetResult(CacheItem value, HybridCacheEntryFlags activeFlags) => SetResult(value, activeFlags, TimeSpan.MaxValue); - private void SetResult(CacheItem value, TimeSpan maxRelativeTime) + private void SetResult(CacheItem value, HybridCacheEntryFlags activeFlags, TimeSpan maxRelativeTime) { - if ((Key.Flags & HybridCacheEntryFlags.DisableLocalCacheWrite) == 0) + if ((activeFlags & HybridCacheEntryFlags.DisableLocalCacheWrite) == 0) { Cache.SetL1(Key.Key, value, _options, maxRelativeTime); // we can do this without a TCS, for SetValue } @@ -536,6 +591,35 @@ private void SetResult(CacheItem value, TimeSpan maxRelativeTime) _ = _result.TrySetResult(value); } } + + private const HybridCacheEntryFlags WriteSideFlags = + HybridCacheEntryFlags.DisableLocalCacheWrite | HybridCacheEntryFlags.DisableDistributedCacheWrite | HybridCacheEntryFlags.DisableCompression; + + /// + /// Applies factory context overrides to the active flags and options after the factory callback has executed. + /// Only write-side flags are applied; read-side flags are ignored (reads already happened). + /// + private static void ApplyFactoryContext(HybridCacheFactoryContext context, ref HybridCacheEntryFlags activeFlags, ref HybridCacheEntryOptions? options) + { + // apply write-side flags from context; read-side flags + // (DisableLocalCacheRead, DisableDistributedCacheRead, DisableUnderlyingData) + // are ignored because reads have already been attempted by this point + if (context.Flags is { } ctxFlags) + { + activeFlags |= ctxFlags & WriteSideFlags; + } + + // apply expiration overrides from context + if (context.Expiration is not null || context.LocalCacheExpiration is not null) + { + options = new HybridCacheEntryOptions + { + Expiration = context.Expiration ?? options?.Expiration, + LocalCacheExpiration = context.LocalCacheExpiration ?? options?.LocalCacheExpiration, + Flags = options?.Flags, + }; + } + } } private static bool ValidateUnicodeCorrectness(ILogger logger, string key, TagSet tags) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs index 93e1e5457cb..811adf9b32e 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs @@ -209,6 +209,83 @@ public override ValueTask RemoveAsync(string key, CancellationToken token = defa return _backendCache is null ? default : new(_backendCache.RemoveAsync(key, token)); } + public override ValueTask GetOrCreateAsync(string key, TState state, + Func> factory, + HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) + { + bool canBeCanceled = cancellationToken.CanBeCanceled; + if (canBeCanceled) + { + cancellationToken.ThrowIfCancellationRequested(); + } + + HybridCacheEntryFlags flags = GetEffectiveFlags(options); + if (!ValidateKey(key)) + { + // we can't use cache, but we can still provide the data + return RunWithoutCacheAsync(flags, state, factory, cancellationToken); + } + + bool eventSourceEnabled = HybridCacheEventSource.Log.IsEnabled(); + + if ((flags & HybridCacheEntryFlags.DisableLocalCacheRead) == 0) + { + if (TryGetExisting(key, out CacheItem? typed) + && typed.TryGetValue(_logger, out T? value)) + { + // short-circuit + if (eventSourceEnabled) + { + HybridCacheEventSource.Log.LocalCacheHit(); + } + + return new(value); + } + else + { + if (eventSourceEnabled) + { + HybridCacheEventSource.Log.LocalCacheMiss(); + } + } + } + + if (GetOrCreateStampedeState(key, flags, out StampedeState? stampede, canBeCanceled, tags)) + { + // new query; we're responsible for making it happen + if (canBeCanceled) + { + // *we* might cancel, but someone else might be depending on the result; start the + // work independently, then we'll with join the outcome + stampede.QueueUserWorkItem(in state, factory, options); + } + else + { + // we're going to run to completion; no need to get complicated + _ = stampede.ExecuteDirectAsync(in state, factory, options); // this larger task includes L2 write etc + return stampede.UnwrapReservedAsync(_logger); + } + } + else + { + // pre-existing query + if (eventSourceEnabled) + { + HybridCacheEventSource.Log.StampedeJoin(); + } + } + + return stampede.JoinAsync(_logger, cancellationToken); + } + + private static ValueTask RunWithoutCacheAsync(HybridCacheEntryFlags flags, TState state, + Func> factory, + CancellationToken cancellationToken) + { + return (flags & HybridCacheEntryFlags.DisableUnderlyingData) == 0 + ? factory(state, new HybridCacheFactoryContext(), cancellationToken) : default; + } + public override ValueTask SetAsync(string key, T value, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken token = default) { // since we're forcing a write: disable L1+L2 read; we'll use a direct pass-thru of the value as the callback, to reuse all the code From 32d849c80635c0392225901475faf44406fb977d Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Tue, 5 May 2026 14:05:24 +0200 Subject: [PATCH 02/16] Added TODO --- .../Internal/DefaultHybridCache.StampedeStateT.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index 8107b6a58c1..e07cc8a80b9 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -376,6 +376,7 @@ private async Task BackgroundFetchAsync() // - we're not writing to L2 // Additionally, if the caller provided an explicit LocalCacheSize via the factory context, // we can also skip serialization for size purposes (we already know the size). + // TODO: The above doesn't make sense as is. CacheItem cacheItem = CacheItem; bool skipSerialize = cacheItem is ImmutableCacheItem && (activeFlags & FlagsDisableL1AndL2Write) == FlagsDisableL1AndL2Write; From b49c1bc548e1654615ab8bdfdfb6fa485e9c9760 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Tue, 2 Jun 2026 16:19:37 +0200 Subject: [PATCH 03/16] DefaultHybridCache: align with approved API (mutable HybridCacheEntryOptions) - Replace removed HybridCacheFactoryContext with HybridCacheEntryOptions in factory signatures. - Pass a clone of caller-provided options to the factory so it can mutate them freely; use Revision to detect mutations. - Honor LocalSize from options (both caller- and factory-set) for L1 writes, including the L2-read path. - Rename internal SetLocalCacheSizeOverride to SetLocalSizeOverride. - Add CloneOptionsOrNew helper using UnsafeAccessor for the internal Clone() on net8+, with a hand-copy fallback for down-level TFMs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DefaultHybridCache.MutableCacheItem.cs | 8 +- .../DefaultHybridCache.StampedeStateT.cs | 97 ++++++++++--------- .../Internal/DefaultHybridCache.cs | 36 ++++++- 3 files changed, 87 insertions(+), 54 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs index 7e71d81b196..36a5d97dcae 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs @@ -13,7 +13,7 @@ internal sealed partial class MutableCacheItem : CacheItem // used to hold private IHybridCacheSerializer? _serializer; private BufferChunk _buffer; private T? _fallbackValue; // only used in the case of serialization failures - private long _localCacheSizeOverride = -1; // -1 means use buffer length + private long _localSizeOverride = -1; // -1 means use buffer length public MutableCacheItem(long creationTimestamp, TagSet tags) : base(creationTimestamp, tags) @@ -67,7 +67,7 @@ public override bool TryGetSize(out long size) // only if we haven't already burned if (TryReserve()) { - size = _localCacheSizeOverride >= 0 ? _localCacheSizeOverride : _buffer.Length; + size = _localSizeOverride >= 0 ? _localSizeOverride : _buffer.Length; _ = Release(); return true; } @@ -76,9 +76,9 @@ public override bool TryGetSize(out long size) return false; } - public void SetLocalCacheSizeOverride(long size) + public void SetLocalSizeOverride(long size) { - _localCacheSizeOverride = size; + _localSizeOverride = size; } public override bool TryReserveBuffer(out BufferChunk buffer) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index e07cc8a80b9..ec20866cb04 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -23,9 +23,9 @@ internal sealed class StampedeState : StampedeState private readonly TaskCompletionSource>? _result; private TState? _state; private Func>? _underlying; // main data factory - private Func>? _underlyingWithContext; // context-aware data factory - private HybridCacheFactoryContext? _factoryContext; + private Func>? _underlyingWithOptions; // options-aware data factory private HybridCacheEntryOptions? _options; + private int _factoryOptionsRevision; // initial Revision of the options instance passed to the factory; used to detect mutations private Task? _sharedUnwrap; // allows multiple non-cancellable callers to share a single task (when no defensive copy needed) // ONLY set the result, without any other side-effects @@ -77,16 +77,18 @@ public Task ExecuteDirectAsync(in TState state, Func> underlying, HybridCacheEntryOptions? options) + public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions? options) { - Debug.Assert(_underlyingWithContext is null, "should not already have factory field"); + Debug.Assert(_underlyingWithOptions is null, "should not already have factory field"); Debug.Assert(underlying is not null, "factory argument should be meaningful"); - // initialize the callback state + // initialize the callback state; we always pass a clone (or fresh instance) to the factory so + // it can mutate the options without affecting the caller's instance, and so we can detect + // mutations via Revision. _state = state; - _underlyingWithContext = underlying; - _factoryContext = new HybridCacheFactoryContext(); - _options = options; + _underlyingWithOptions = underlying; + _options = CloneOptionsOrNew(options); + _factoryOptionsRevision = _options.Revision; #if NETCOREAPP3_0_OR_GREATER ThreadPool.UnsafeQueueUserWorkItem(this, false); @@ -96,16 +98,16 @@ public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions? options) + public Task ExecuteDirectAsync(in TState state, Func> underlying, HybridCacheEntryOptions? options) { - Debug.Assert(_underlyingWithContext is null, "should not already have factory field"); + Debug.Assert(_underlyingWithOptions is null, "should not already have factory field"); Debug.Assert(underlying is not null, "factory argument should be meaningful"); - // initialize the callback state + // initialize the callback state (see QueueUserWorkItem above for cloning rationale) _state = state; - _underlyingWithContext = underlying; - _factoryContext = new HybridCacheFactoryContext(); - _options = options; + _underlyingWithOptions = underlying; + _options = CloneOptionsOrNew(options); + _factoryOptionsRevision = _options.Revision; return BackgroundFetchAsync(); } @@ -300,9 +302,9 @@ private async Task BackgroundFetchAsync() HybridCacheEventSource.Log.UnderlyingDataQueryStart(); } - if (_underlyingWithContext is not null) + if (_underlyingWithOptions is not null) { - newValue = await _underlyingWithContext(_state!, _factoryContext!, SharedToken).ConfigureAwait(false); + newValue = await _underlyingWithOptions(_state!, _options!, SharedToken).ConfigureAwait(false); } else { @@ -331,10 +333,11 @@ private async Task BackgroundFetchAsync() throw; } - // apply context overrides set by the factory callback - if (_factoryContext is not null) + // honor any mutations the factory made to the options it received; we use Revision + // as a fast-path skip for the common case where the factory didn't touch them. + if (_underlyingWithOptions is not null && _options!.Revision != _factoryOptionsRevision) { - ApplyFactoryContext(_factoryContext, ref activeFlags, ref _options); + ApplyFactoryOptions(_options, ref activeFlags); } // check whether we're going to hit a timing problem with tag invalidation @@ -374,7 +377,7 @@ private async Task BackgroundFetchAsync() // Rephrasing that: the only scenario in which we *do not* need to serialize is if: // - it is an ImmutableCacheItem (so we don't need bytes for the CacheItem, L1) // - we're not writing to L2 - // Additionally, if the caller provided an explicit LocalCacheSize via the factory context, + // Additionally, if the caller (or factory) provided an explicit LocalSize via the entry options, // we can also skip serialization for size purposes (we already know the size). // TODO: The above doesn't make sense as is. CacheItem cacheItem = CacheItem; @@ -491,13 +494,14 @@ private void SetResultAndRecycleIfAppropriate(ref BufferChunk value, TimeSpan re // set a result from L2 cache Debug.Assert(value.OversizedArray is not null, "expected buffer"); + long? localSizeOverride = ResolveLocalSize(); IHybridCacheSerializer serializer = Cache.GetSerializer(); CacheItem cacheItem; switch (CacheItem) { case ImmutableCacheItem immutable: // deserialize; and store object; buffer can be recycled now - immutable.SetValue(serializer.Deserialize(new(value.OversizedArray!, value.Offset, value.Length)), value.Length); + immutable.SetValue(serializer.Deserialize(new(value.OversizedArray!, value.Offset, value.Length)), localSizeOverride ?? value.Length); value.RecycleIfAppropriate(); cacheItem = immutable; break; @@ -505,6 +509,11 @@ private void SetResultAndRecycleIfAppropriate(ref BufferChunk value, TimeSpan re // use the buffer directly as the backing in the cache-item; do *not* recycle now mutable.SetValue(ref value, serializer); mutable.DebugOnlyTrackBuffer(Cache); + if (localSizeOverride is { } mutableSize) + { + mutable.SetLocalSizeOverride(mutableSize); + } + cacheItem = mutable; break; default: @@ -515,6 +524,13 @@ private void SetResultAndRecycleIfAppropriate(ref BufferChunk value, TimeSpan re SetResult(cacheItem, Key.Flags, remainingTime); } + private long? ResolveLocalSize() + { + // factory mutations are applied directly to _options (which is a clone). null means "use + // implementation default", per the LocalSize API contract. + return _options?.LocalSize; + } + private void SetImmutableResultWithoutSerialize(T value, HybridCacheEntryFlags activeFlags) { Debug.Assert((activeFlags & FlagsDisableL1AndL2Write) == FlagsDisableL1AndL2Write, "Only expected if L1+L2 disabled"); @@ -525,7 +541,7 @@ private void SetImmutableResultWithoutSerialize(T value, HybridCacheEntryFlags a { case ImmutableCacheItem immutable: // no serialize needed - immutable.SetValue(value, size: _factoryContext?.LocalCacheSize ?? -1); + immutable.SetValue(value, size: ResolveLocalSize() ?? -1); cacheItem = immutable; break; default: @@ -545,7 +561,7 @@ private void SetResultPreSerialized(T value, ref BufferChunk buffer, IHybridCach { case ImmutableCacheItem immutable: // no serialize needed - immutable.SetValue(value, size: _factoryContext?.LocalCacheSize ?? buffer.Length); + immutable.SetValue(value, size: ResolveLocalSize() ?? buffer.Length); cacheItem = immutable; // (but leave the buffer alone) @@ -562,9 +578,9 @@ private void SetResultPreSerialized(T value, ref BufferChunk buffer, IHybridCach mutable.DebugOnlyTrackBuffer(Cache); } - if (_factoryContext?.LocalCacheSize is { } mutableSize) + if (ResolveLocalSize() is { } mutableSize) { - mutable.SetLocalCacheSizeOverride(mutableSize); + mutable.SetLocalSizeOverride(mutableSize); } cacheItem = mutable; @@ -597,29 +613,18 @@ private void SetResult(CacheItem value, HybridCacheEntryFlags activeFlags, Ti HybridCacheEntryFlags.DisableLocalCacheWrite | HybridCacheEntryFlags.DisableDistributedCacheWrite | HybridCacheEntryFlags.DisableCompression; /// - /// Applies factory context overrides to the active flags and options after the factory callback has executed. - /// Only write-side flags are applied; read-side flags are ignored (reads already happened). + /// Applies factory mutations to the active flags after the factory callback has executed. + /// Only write-side flags are honored; read-side flags are ignored (reads already happened). + /// Expiration / LocalCacheExpiration / LocalSize mutations need no action here because the + /// factory wrote directly to , which is read by SetL1 / SetL2Async / ResolveLocalSize. /// - private static void ApplyFactoryContext(HybridCacheFactoryContext context, ref HybridCacheEntryFlags activeFlags, ref HybridCacheEntryOptions? options) + private static void ApplyFactoryOptions(HybridCacheEntryOptions factoryOptions, ref HybridCacheEntryFlags activeFlags) { - // apply write-side flags from context; read-side flags - // (DisableLocalCacheRead, DisableDistributedCacheRead, DisableUnderlyingData) - // are ignored because reads have already been attempted by this point - if (context.Flags is { } ctxFlags) - { - activeFlags |= ctxFlags & WriteSideFlags; - } - - // apply expiration overrides from context - if (context.Expiration is not null || context.LocalCacheExpiration is not null) - { - options = new HybridCacheEntryOptions - { - Expiration = context.Expiration ?? options?.Expiration, - LocalCacheExpiration = context.LocalCacheExpiration ?? options?.LocalCacheExpiration, - Flags = options?.Flags, - }; - } + // Replace write-side bits with whatever the factory left behind; preserve everything else + // (including read-side flags already acted on, plus any constraints we OR'd in earlier such + // as DisableDistributedCache from validation). + HybridCacheEntryFlags factoryFlags = factoryOptions.Flags ?? HybridCacheEntryFlags.None; + activeFlags = (activeFlags & ~WriteSideFlags) | (factoryFlags & WriteSideFlags); } } diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs index 811adf9b32e..382b20995fd 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs @@ -210,7 +210,7 @@ public override ValueTask RemoveAsync(string key, CancellationToken token = defa } public override ValueTask GetOrCreateAsync(string key, TState state, - Func> factory, + Func> factory, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) { bool canBeCanceled = cancellationToken.CanBeCanceled; @@ -223,7 +223,7 @@ public override ValueTask GetOrCreateAsync(string key, TState stat if (!ValidateKey(key)) { // we can't use cache, but we can still provide the data - return RunWithoutCacheAsync(flags, state, factory, cancellationToken); + return RunWithoutCacheAsync(flags, state, factory, options, cancellationToken); } bool eventSourceEnabled = HybridCacheEventSource.Log.IsEnabled(); @@ -279,11 +279,39 @@ public override ValueTask GetOrCreateAsync(string key, TState stat } private static ValueTask RunWithoutCacheAsync(HybridCacheEntryFlags flags, TState state, - Func> factory, + Func> factory, + HybridCacheEntryOptions? options, CancellationToken cancellationToken) { + // pass a clone (or fresh instance) so the caller's options are never mutated; + // there's no cache to honor the mutations against, but the factory may rely on + // being able to mutate the parameter without surprising the caller. return (flags & HybridCacheEntryFlags.DisableUnderlyingData) == 0 - ? factory(state, new HybridCacheFactoryContext(), cancellationToken) : default; + ? factory(state, CloneOptionsOrNew(options), cancellationToken) : default; + } + + internal static HybridCacheEntryOptions CloneOptionsOrNew(HybridCacheEntryOptions? options) + { + if (options is null) + { + return new HybridCacheEntryOptions(); + } + +#if NET8_0_OR_GREATER + return Clone(options); + + [UnsafeAccessor(UnsafeAccessorKind.Method, Name = nameof(Clone))] + extern static HybridCacheEntryOptions Clone(HybridCacheEntryOptions options); +#else + // Down-level TFMs cannot reach the internal Clone(); copy by hand. + return new HybridCacheEntryOptions + { + Expiration = options.Expiration, + LocalCacheExpiration = options.LocalCacheExpiration, + Flags = options.Flags, + LocalSize = options.LocalSize, + }; +#endif } public override ValueTask SetAsync(string key, T value, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken token = default) From 15d649097d173ad70f1774fb7322d923eefd3b9a Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Tue, 2 Jun 2026 18:37:13 +0200 Subject: [PATCH 04/16] Persist LocalSize into L2 payload so factory-set values survive L1 eviction When a factory sets HybridCacheEntryOptions.LocalSize, that value lived only on the in-memory cache entry. After L1 eviction, an L2 hit would rehydrate the entry without the original LocalSize, falling back to the buffer length. Introduce a v2 payload format that carries an optional LocalSize varint (gated by a new PayloadFlags.HasLocalSize bit). The writer emits v2 only when a LocalSize override is being persisted; v1 remains for the common case, so upgrades don't invalidate existing L2 entries and older readers can still read newer writers' output when no override is used. Older readers reject v2 (cache miss). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Internal/DefaultHybridCache.L2.cs | 2 +- .../DefaultHybridCache.StampedeStateT.cs | 9 +- .../Internal/HybridCachePayload.cs | 284 +++++++++++------- .../PayloadTests.cs | 82 ++++- 4 files changed, 248 insertions(+), 129 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.L2.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.L2.cs index 4293d54bc30..6cc340b8609 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.L2.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.L2.cs @@ -225,7 +225,7 @@ private async ValueTask WritePayloadAsync(string key, CacheItem cacheItem, Buffe byte[] oversized = ArrayPool.Shared.Rent(maxLength); int length = HybridCachePayload.Write(oversized, key, cacheItem.CreationTimestamp, GetL2AbsoluteExpirationRelativeToNow(options), - HybridCachePayload.PayloadFlags.None, cacheItem.Tags, payload.AsSequence()); + HybridCachePayload.PayloadFlags.None, cacheItem.Tags, payload.AsSequence(), localCacheSize: options?.LocalSize); await SetDirectL2Async(key, new(oversized, 0, length, true), GetL2DistributedCacheOptions(options), token).ConfigureAwait(false); diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index ec20866cb04..023e6da7b72 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -262,7 +262,7 @@ private async Task BackgroundFetchAsync() // result is the wider payload including HC headers; unwrap it: HybridCachePayload.HybridCachePayloadParseResult parseResult = HybridCachePayload.TryParse( result.AsArraySegment(), Key.Key, CacheItem.Tags, Cache, out ArraySegment payload, out TimeSpan remainingTime, - out HybridCachePayload.PayloadFlags flags, out ushort entropy, out TagSet pendingTags, out Exception? fault); + out HybridCachePayload.PayloadFlags flags, out ushort entropy, out TagSet pendingTags, out long? payloadLocalSize, out Exception? fault); switch (parseResult) { case HybridCachePayload.HybridCachePayloadParseResult.Success: @@ -271,7 +271,7 @@ private async Task BackgroundFetchAsync() { // move into the payload segment (minus any framing/header/etc data) result = new(payload.Array!, payload.Offset, payload.Count, result.ReturnToPool); - SetResultAndRecycleIfAppropriate(ref result, remainingTime); + SetResultAndRecycleIfAppropriate(ref result, remainingTime, payloadLocalSize); return; } @@ -489,12 +489,13 @@ private void SetDefaultResult() } } - private void SetResultAndRecycleIfAppropriate(ref BufferChunk value, TimeSpan remainingTime) + private void SetResultAndRecycleIfAppropriate(ref BufferChunk value, TimeSpan remainingTime, long? payloadLocalSize = null) { // set a result from L2 cache Debug.Assert(value.OversizedArray is not null, "expected buffer"); - long? localSizeOverride = ResolveLocalSize(); + // precedence: size persisted in the payload > caller-provided options.LocalSize > buffer.Length + long? localSizeOverride = payloadLocalSize ?? ResolveLocalSize(); IHybridCacheSerializer serializer = Cache.GetSerializer(); CacheItem cacheItem; switch (CacheItem) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs index 9199a5cfd15..f36a5c8d47e 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs @@ -11,29 +11,40 @@ namespace Microsoft.Extensions.Caching.Hybrid.Internal; // logic related to the payload that we send to IDistributedCache internal static class HybridCachePayload { - // FORMAT (v1): - // fixed-size header (so that it can be reliably broadcast) + // FORMAT (v2): + // fixed-size header (so that it can be reliably broadcast) // 2 bytes: sentinel+version - // 2 bytes: entropy (this is a random, and is to help with multi-node collisions at the same time) + // 2 bytes: entropy (random, to help with multi-node collisions at the same time) // 8 bytes: creation time (UTC ticks, little-endian) // and the dynamic part // varint: flags (little-endian) // varint: payload size // varint: duration (ticks relative to creation time) + // (when PayloadFlags.HasLocalSize is set): varint: local cache size // varint: tag count // varint+utf8: key // (for each tag): varint+utf8: tagN // (payload-size bytes): payload // 2 bytes: sentinel+version (repeated, for reliability) // (at this point, all bytes *must* be exhausted, or it is treated as failure) - - // the encoding for varint etc is akin to BinaryWriter, also comparable to FormatterBinaryWriter in OutputCaching + // + // FORMAT (v1): identical to v2 but predates the optional local-size varint. + // Writers currently emit v1 when no LocalSize override is + // being persisted, so existing L2 entries remain readable across upgrades and v1-only readers + // can still read newer writers' output in the common case. v2 is emitted when LocalSize is + // being persisted; older readers reject v2 (FormatNotRecognized -> cache miss). + // + // The expectation is that readers will support reading all old formats + // (at least as long as they stay easily backwards compatible), while writers will write a couple + // of the latest formats, to make switching versions easier, while not accumulating too much cruft in the code. private const int MaxVarint64Length = 10; private const byte SentinelPrefix = 0x03; - private const byte ProtocolVersion = 0x01; - private const ushort UInt16SentinelPrefixPair = (ProtocolVersion << 8) | SentinelPrefix; + private const byte ProtocolVersion1 = 0x01; + private const byte ProtocolVersion2 = 0x02; + private const ushort UInt16SentinelPrefixPairV1 = (ProtocolVersion1 << 8) | SentinelPrefix; + private const ushort UInt16SentinelPrefixPairV2 = (ProtocolVersion2 << 8) | SentinelPrefix; private static readonly Random _entropySource = new(); // doesn't need to be cryptographic @@ -42,6 +53,10 @@ internal static class HybridCachePayload internal enum PayloadFlags : uint { None = 0, + + // When set, the dynamic part carries an additional varint encoding a per-entry + // local cache size override. + HasLocalSize = 1, } internal enum HybridCachePayloadParseResult @@ -67,6 +82,7 @@ public static int GetMaxBytes(string key, TagSet tags, int payloadSize) + MaxVarint64Length // flags + MaxVarint64Length // payload size + MaxVarint64Length // duration + + MaxVarint64Length // optional local cache size (always budgeted; trivial over-estimate when absent) + MaxVarint64Length // tag count + 2 // trailing sentinel + version + GetMaxStringLength(key.Length) // key @@ -99,11 +115,35 @@ static int GetMaxStringLength(int charCount) => [System.Diagnostics.CodeAnalysis.SuppressMessage("Security", "CA5394:Do not use insecure randomness", Justification = "Not cryptographic")] public static int Write(byte[] destination, - string key, long creationTime, TimeSpan duration, PayloadFlags flags, TagSet tags, ReadOnlySequence payload) + string key, long creationTime, TimeSpan duration, PayloadFlags flags, TagSet tags, ReadOnlySequence payload, + long? localCacheSize = null) { int payloadLength = checked((int)payload.Length); - BinaryPrimitives.WriteUInt16LittleEndian(destination.AsSpan(0, 2), UInt16SentinelPrefixPair); + // a negative localCacheSize is the documented "reset to default" sentinel - treat it as absent. + if (localCacheSize is < 0) + { + localCacheSize = null; + } + + // v2 is used if we are persisting a LocalSize override; otherwise stay on v1 so that + // upgrades don't invalidate every existing L2 entry and older readers can still read + // entries written by newer writers when no override is used. + ushort sentinel; + if (localCacheSize is null) + { + sentinel = UInt16SentinelPrefixPairV1; + + // defensive: never persist a stale HasLocalSize bit without an accompanying value + flags &= ~PayloadFlags.HasLocalSize; + } + else + { + sentinel = UInt16SentinelPrefixPairV2; + flags |= PayloadFlags.HasLocalSize; + } + + BinaryPrimitives.WriteUInt16LittleEndian(destination.AsSpan(0, 2), sentinel); BinaryPrimitives.WriteUInt16LittleEndian(destination.AsSpan(2, 2), (ushort)_entropySource.Next(0, 0x010000)); // Next is exclusive at RHS BinaryPrimitives.WriteInt64LittleEndian(destination.AsSpan(4, 8), creationTime); int len = 12; @@ -117,6 +157,11 @@ public static int Write(byte[] destination, Write7BitEncodedInt64(destination, ref len, (uint)flags); Write7BitEncodedInt64(destination, ref len, (ulong)payloadLength); Write7BitEncodedInt64(destination, ref len, (ulong)durationTicks); + if (localCacheSize is { } size) + { + Write7BitEncodedInt64(destination, ref len, (ulong)size); + } + Write7BitEncodedInt64(destination, ref len, (ulong)tags.Count); WriteString(destination, ref len, key); switch (tags.Count) @@ -137,7 +182,7 @@ public static int Write(byte[] destination, payload.CopyTo(destination.AsSpan(len, payloadLength)); len += payloadLength; - BinaryPrimitives.WriteUInt16LittleEndian(destination.AsSpan(len, 2), UInt16SentinelPrefixPair); + BinaryPrimitives.WriteUInt16LittleEndian(destination.AsSpan(len, 2), sentinel); return len + 2; static void Write7BitEncodedInt64(byte[] target, ref int offset, ulong value) @@ -171,7 +216,7 @@ static void WriteString(byte[] target, ref int offset, string value) "SA1122:Use string.Empty for empty strings", Justification = "Subjective, but; ugly")] [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1204:Static elements should appear before instance elements", Justification = "False positive?")] public static HybridCachePayloadParseResult TryParse(ArraySegment source, string key, TagSet knownTags, DefaultHybridCache cache, - out ArraySegment payload, out TimeSpan remainingTime, out PayloadFlags flags, out ushort entropy, out TagSet pendingTags, out Exception? fault) + out ArraySegment payload, out TimeSpan remainingTime, out PayloadFlags flags, out ushort entropy, out TagSet pendingTags, out long? localCacheSize, out Exception? fault) { fault = null; @@ -180,6 +225,7 @@ public static HybridCachePayloadParseResult TryParse(ArraySegment source, payload = default; flags = 0; remainingTime = TimeSpan.Zero; + localCacheSize = null; string[] pendingTagBuffer = []; int pendingTagsCount = 0; @@ -194,128 +240,142 @@ public static HybridCachePayloadParseResult TryParse(ArraySegment source, char[] scratch = []; try { - switch (BinaryPrimitives.ReadUInt16LittleEndian(bytes)) + ushort sentinel = BinaryPrimitives.ReadUInt16LittleEndian(bytes); + switch (sentinel) { - case UInt16SentinelPrefixPair: - entropy = BinaryPrimitives.ReadUInt16LittleEndian(bytes.Slice(2)); - long creationTime = BinaryPrimitives.ReadInt64LittleEndian(bytes.Slice(4)); - bytes = bytes.Slice(12); // the end of the fixed part + case UInt16SentinelPrefixPairV1: + case UInt16SentinelPrefixPairV2: + break; + default: + return HybridCachePayloadParseResult.FormatNotRecognized; + } - if (cache.IsWildcardExpired(creationTime)) - { - return HybridCachePayloadParseResult.ExpiredByWildcard; - } + entropy = BinaryPrimitives.ReadUInt16LittleEndian(bytes.Slice(2)); + long creationTime = BinaryPrimitives.ReadInt64LittleEndian(bytes.Slice(4)); + bytes = bytes.Slice(12); // the end of the fixed part - if (!TryRead7BitEncodedInt64(ref bytes, out ulong u64)) // flags - { - return HybridCachePayloadParseResult.InvalidData; - } + if (cache.IsWildcardExpired(creationTime)) + { + return HybridCachePayloadParseResult.ExpiredByWildcard; + } - flags = (PayloadFlags)u64; + if (!TryRead7BitEncodedInt64(ref bytes, out ulong u64)) // flags + { + return HybridCachePayloadParseResult.InvalidData; + } - if (!TryRead7BitEncodedInt64(ref bytes, out u64) || u64 > int.MaxValue) // payload length - { - return HybridCachePayloadParseResult.InvalidData; - } + flags = (PayloadFlags)u64; - int payloadLength = (int)u64; + if (!TryRead7BitEncodedInt64(ref bytes, out u64) || u64 > int.MaxValue) // payload length + { + return HybridCachePayloadParseResult.InvalidData; + } - if (!TryRead7BitEncodedInt64(ref bytes, out ulong duration)) // duration - { - return HybridCachePayloadParseResult.InvalidData; - } + int payloadLength = (int)u64; - var remainingTicks = (creationTime + (long)duration) - now; - if (remainingTicks <= 0) - { - return HybridCachePayloadParseResult.ExpiredByEntry; - } + if (!TryRead7BitEncodedInt64(ref bytes, out ulong duration)) // duration + { + return HybridCachePayloadParseResult.InvalidData; + } - remainingTime = DefaultHybridCache.TicksToTimeSpan(remainingTicks); + var remainingTicks = (creationTime + (long)duration) - now; + if (remainingTicks <= 0) + { + return HybridCachePayloadParseResult.ExpiredByEntry; + } - if (!TryRead7BitEncodedInt64(ref bytes, out u64) || u64 > int.MaxValue) // tag count - { - return HybridCachePayloadParseResult.InvalidData; - } + remainingTime = DefaultHybridCache.TicksToTimeSpan(remainingTicks); + + if ((flags & PayloadFlags.HasLocalSize) != 0) + { + if (!TryRead7BitEncodedInt64(ref bytes, out ulong sizeU64) || sizeU64 > long.MaxValue) + { + return HybridCachePayloadParseResult.InvalidData; + } - int tagCount = (int)u64; + localCacheSize = (long)sizeU64; + } - if (!TryReadString(ref bytes, ref scratch, out ReadOnlySpan stringSpan)) - { - return HybridCachePayloadParseResult.InvalidData; - } + if (!TryRead7BitEncodedInt64(ref bytes, out u64) || u64 > int.MaxValue) // tag count + { + return HybridCachePayloadParseResult.InvalidData; + } - if (!stringSpan.SequenceEqual(key.AsSpan())) - { - return HybridCachePayloadParseResult.InvalidKey; // key must match! - } + int tagCount = (int)u64; - for (int i = 0; i < tagCount; i++) - { - if (!TryReadString(ref bytes, ref scratch, out stringSpan)) - { - return HybridCachePayloadParseResult.InvalidData; - } - - bool isTagExpired; - bool isPending; - if (knownTags.TryFind(stringSpan, out string? tagString)) - { - // prefer to re-use existing tag strings when they exist - isTagExpired = cache.IsTagExpired(tagString, creationTime, out isPending); - } - else - { - // if an unknown tag; we might need to juggle - isTagExpired = cache.IsTagExpired(stringSpan, creationTime, out isPending); - } - - if (isPending) - { - // might be expired, but the operation is still in-flight - if (pendingTagsCount == pendingTagBuffer.Length) - { - string[] newBuffer = ArrayPool.Shared.Rent(Math.Max(4, pendingTagsCount * 2)); - pendingTagBuffer.CopyTo(newBuffer, 0); - ArrayPool.Shared.Return(pendingTagBuffer); - pendingTagBuffer = newBuffer; - } - - pendingTagBuffer[pendingTagsCount++] = tagString ?? stringSpan.ToString(); - } - else if (isTagExpired) - { - // definitely an expired tag - return HybridCachePayloadParseResult.ExpiredByTag; - } - } + if (!TryReadString(ref bytes, ref scratch, out ReadOnlySpan stringSpan)) + { + return HybridCachePayloadParseResult.InvalidData; + } - if (bytes.Length != payloadLength + 2 - || BinaryPrimitives.ReadUInt16LittleEndian(bytes.Slice(payloadLength)) != UInt16SentinelPrefixPair) - { - return HybridCachePayloadParseResult.InvalidData; - } + if (!stringSpan.SequenceEqual(key.AsSpan())) + { + return HybridCachePayloadParseResult.InvalidKey; // key must match! + } + + for (int i = 0; i < tagCount; i++) + { + if (!TryReadString(ref bytes, ref scratch, out stringSpan)) + { + return HybridCachePayloadParseResult.InvalidData; + } - int start = source.Offset + source.Count - (payloadLength + 2); - payload = new(source.Array!, start, payloadLength); + bool isTagExpired; + bool isPending; + if (knownTags.TryFind(stringSpan, out string? tagString)) + { + // prefer to re-use existing tag strings when they exist + isTagExpired = cache.IsTagExpired(tagString, creationTime, out isPending); + } + else + { + // if an unknown tag; we might need to juggle + isTagExpired = cache.IsTagExpired(stringSpan, creationTime, out isPending); + } - // finalize the pending tag buffer (in-flight tag expirations) - switch (pendingTagsCount) + if (isPending) + { + // might be expired, but the operation is still in-flight + if (pendingTagsCount == pendingTagBuffer.Length) { - case 0: - break; - case 1: - pendingTags = new(pendingTagBuffer[0]); - break; - default: - pendingTags = new(pendingTagBuffer.AsSpan(0, pendingTagsCount).ToArray()); - break; + string[] newBuffer = ArrayPool.Shared.Rent(Math.Max(4, pendingTagsCount * 2)); + pendingTagBuffer.CopyTo(newBuffer, 0); + ArrayPool.Shared.Return(pendingTagBuffer); + pendingTagBuffer = newBuffer; } - return HybridCachePayloadParseResult.Success; + pendingTagBuffer[pendingTagsCount++] = tagString ?? stringSpan.ToString(); + } + else if (isTagExpired) + { + // definitely an expired tag + return HybridCachePayloadParseResult.ExpiredByTag; + } + } + + if (bytes.Length != payloadLength + 2 + || BinaryPrimitives.ReadUInt16LittleEndian(bytes.Slice(payloadLength)) != sentinel) + { + return HybridCachePayloadParseResult.InvalidData; + } + + int start = source.Offset + source.Count - (payloadLength + 2); + payload = new(source.Array!, start, payloadLength); + + // finalize the pending tag buffer (in-flight tag expirations) + switch (pendingTagsCount) + { + case 0: + break; + case 1: + pendingTags = new(pendingTagBuffer[0]); + break; default: - return HybridCachePayloadParseResult.FormatNotRecognized; + pendingTags = new(pendingTagBuffer.AsSpan(0, pendingTagsCount).ToArray()); + break; } + + return HybridCachePayloadParseResult.Success; } catch (Exception ex) { @@ -403,7 +463,7 @@ static bool TryRead7BitEncodedInt64(ref ReadOnlySpan buffer, out ulong res byteReadJustNow = buffer[index++]; if (byteReadJustNow > 0b_1u) { - throw new OverflowException(); + return false; } result |= (ulong)byteReadJustNow << (MaxBytesWithoutOverflow * 7); diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs index 6ee4a8a5558..256f8298b0d 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers; @@ -54,13 +54,71 @@ public void RoundTrip_Success(string delimitedTags, int expectedLength, int tagC Assert.Equal(expectedLength, actualLength); clock.Add(TimeSpan.FromSeconds(10)); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); log.WriteLine($"Entropy: {entropy}; Flags: {flags}"); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); Assert.True(payload.SequenceEqual(bytes)); Assert.True(pendingTags.IsEmpty); } + [Theory] + [InlineData(null)] + [InlineData(0L)] + [InlineData(42L)] + [InlineData(long.MaxValue)] + public void RoundTrip_LocalCacheSize(long? localCacheSize) + { + using var provider = GetDefaultCache(out var cache); + + byte[] bytes = new byte[64]; + new Random().NextBytes(bytes); + + string key = "k"; + TagSet tags = TagSet.Empty; + int maxLen = HybridCachePayload.GetMaxBytes(key, tags, bytes.Length); + byte[] oversized = ArrayPool.Shared.Rent(maxLen); + + int actualLength = HybridCachePayload.Write(oversized, key, cache.CurrentTimestamp(), TimeSpan.FromMinutes(1), 0, tags, new(bytes), localCacheSize: localCacheSize); + + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, + out var payload, out _, out var flags, out _, out var pendingTags, out long? parsedSize, out _); + + Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); + Assert.True(payload.SequenceEqual(bytes)); + Assert.True(pendingTags.IsEmpty); + Assert.Equal(localCacheSize, parsedSize); + Assert.Equal(localCacheSize is not null, (flags & HybridCachePayload.PayloadFlags.HasLocalSize) != 0); + + ArrayPool.Shared.Return(oversized); + } + + [Theory] + [InlineData(-1L)] + [InlineData(-100L)] + public void RoundTrip_LocalCacheSize_NegativeSentinelIsNormalizedToAbsent(long localCacheSize) + { + // negative LocalSize is the documented "reset to default" sentinel; Write must drop it + // so the payload is emitted as v1 with no HasLocalSize flag. + using var provider = GetDefaultCache(out var cache); + + byte[] bytes = new byte[8]; + string key = "k"; + TagSet tags = TagSet.Empty; + int maxLen = HybridCachePayload.GetMaxBytes(key, tags, bytes.Length); + byte[] oversized = ArrayPool.Shared.Rent(maxLen); + + int actualLength = HybridCachePayload.Write(oversized, key, cache.CurrentTimestamp(), TimeSpan.FromMinutes(1), 0, tags, new(bytes), localCacheSize: localCacheSize); + + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, + out _, out _, out var flags, out _, out _, out long? parsedSize, out _); + + Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); + Assert.Null(parsedSize); + Assert.Equal(HybridCachePayload.PayloadFlags.None, flags & HybridCachePayload.PayloadFlags.HasLocalSize); + + ArrayPool.Shared.Return(oversized); + } + [Fact] public void RoundTrip_SelfExpiration() { @@ -83,13 +141,13 @@ public void RoundTrip_SelfExpiration() Assert.Equal(1063, actualLength); clock.Add(TimeSpan.FromSeconds(58)); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); Assert.True(payload.SequenceEqual(bytes)); Assert.True(pendingTags.IsEmpty); clock.Add(TimeSpan.FromSeconds(4)); - result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out payload, out remaining, out flags, out entropy, out pendingTags, out _); + result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out payload, out remaining, out flags, out entropy, out pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.ExpiredByEntry, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -119,7 +177,7 @@ public async Task RoundTrip_WildcardExpiration() clock.Add(TimeSpan.FromSeconds(2)); await cache.RemoveByTagAsync("*"); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.ExpiredByWildcard, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -149,13 +207,13 @@ public async Task RoundTrip_TagExpiration() clock.Add(TimeSpan.FromSeconds(2)); await cache.RemoveByTagAsync("other_tag"); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); Assert.True(payload.SequenceEqual(bytes)); Assert.True(pendingTags.IsEmpty); await cache.RemoveByTagAsync("some_tag"); - result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out payload, out remaining, out flags, out entropy, out pendingTags, out _); + result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out payload, out remaining, out flags, out entropy, out pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.ExpiredByTag, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -187,7 +245,7 @@ public async Task RoundTrip_TagExpiration_Pending() var tcs = new TaskCompletionSource(); cache.DebugInvalidateTag("some_tag", tcs.Task); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); Assert.True(payload.SequenceEqual(bytes)); Assert.Equal(1, pendingTags.Count); @@ -209,7 +267,7 @@ public void Gibberish() byte[] bytes = new byte[1024]; new Random().NextBytes(bytes); - var result = HybridCachePayload.TryParse(new(bytes), "whatever", TagSet.Empty, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + var result = HybridCachePayload.TryParse(new(bytes), "whatever", TagSet.Empty, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.FormatNotRecognized, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -236,7 +294,7 @@ public void RoundTrip_Truncated() log.WriteLine($"bytes written: {actualLength}"); Assert.Equal(1063, actualLength); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength - 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength - 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.InvalidData, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -263,7 +321,7 @@ public void RoundTrip_Oversized() log.WriteLine($"bytes written: {actualLength}"); Assert.Equal(1063, actualLength); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength + 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength + 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.InvalidData, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -358,7 +416,7 @@ public void RoundTrip_WithPendingTags_WhenKnownTagsMismatch(string delimitedTags // Parse with empty knownTags to force all tags into pendingTags via the rented buffer path var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, TagSet.Empty, cache, - out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _); + out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); Assert.True(payload.SequenceEqual(bytes)); Assert.Equal(tagCount, pendingTags.Count); From 80fe2976f46951b9610a9cf01b1555697e3fa9c7 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Wed, 3 Jun 2026 15:57:38 +0200 Subject: [PATCH 05/16] Skip serialization when LocalSize is known and L2 write is disabled For an ImmutableCacheItem going only to L1, the sole reason to serialize was to know the byte length to report to MemoryCache (for SizeLimit accounting). When the caller or factory has set HybridCacheEntryOptions.LocalSize, the size is already known, so the serialization round-trip can be skipped entirely. Also rewrites the surrounding comment to enumerate the serialization-required cases in one pass instead of using a primary rule plus an "additionally" note. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DefaultHybridCache.StampedeStateT.cs | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index 023e6da7b72..394cb6fe0e6 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -18,7 +18,6 @@ internal partial class DefaultHybridCache internal sealed class StampedeState : StampedeState { // note on terminology: L1 and L2 are, for brevity, used interchangeably with "local" and "distributed" cache, i.e. `IMemoryCache` and `IDistributedCache` - private const HybridCacheEntryFlags FlagsDisableL1AndL2Write = HybridCacheEntryFlags.DisableLocalCacheWrite | HybridCacheEntryFlags.DisableDistributedCacheWrite; private readonly TaskCompletionSource>? _result; private TState? _state; @@ -369,19 +368,21 @@ private async Task BackgroundFetchAsync() CacheItem.UnsafeSetCreationTimestamp(time); } - // If we're writing this value *anywhere*, we're going to need to serialize; this is obvious - // in the case of L2, but we also need it for L1, because MemoryCache might be enforcing - // SizeLimit (we can't know - it is an abstraction), and for *that* we need to know the item size. - // Likewise, if we're writing to a MutableCacheItem, we'll be serializing *anyway* for the payload. - // - // Rephrasing that: the only scenario in which we *do not* need to serialize is if: - // - it is an ImmutableCacheItem (so we don't need bytes for the CacheItem, L1) - // - we're not writing to L2 - // Additionally, if the caller (or factory) provided an explicit LocalSize via the entry options, - // we can also skip serialization for size purposes (we already know the size). - // TODO: The above doesn't make sense as is. + // Serialization is required in any of these cases: + // - the CacheItem is mutable (the serialized bytes are the L1 and stampede storage, since each + // read deserializes a defensive copy, and they double as the L2 payload); + // - the CacheItem is immutable but we are writing to L2 (serialization produces + // the payload); + // - the CacheItem is immutable, we are writing to L1, and the entry size is not + // known up-front (MemoryCache may enforce SizeLimit, and we report the + // serialized byte length as the size). When the caller or factory has supplied + // LocalSize via the entry options the size is already known, so this last + // case does not apply. CacheItem cacheItem = CacheItem; - bool skipSerialize = cacheItem is ImmutableCacheItem && (activeFlags & FlagsDisableL1AndL2Write) == FlagsDisableL1AndL2Write; + long? knownLocalSize = ResolveLocalSize(); + bool skipSerialize = cacheItem is ImmutableCacheItem + && (activeFlags & HybridCacheEntryFlags.DisableDistributedCacheWrite) != 0 + && ((activeFlags & HybridCacheEntryFlags.DisableLocalCacheWrite) != 0 || knownLocalSize is not null); if (skipSerialize) { @@ -534,7 +535,10 @@ private void SetResultAndRecycleIfAppropriate(ref BufferChunk value, TimeSpan re private void SetImmutableResultWithoutSerialize(T value, HybridCacheEntryFlags activeFlags) { - Debug.Assert((activeFlags & FlagsDisableL1AndL2Write) == FlagsDisableL1AndL2Write, "Only expected if L1+L2 disabled"); + Debug.Assert( + (activeFlags & HybridCacheEntryFlags.DisableDistributedCacheWrite) != 0 + && ((activeFlags & HybridCacheEntryFlags.DisableLocalCacheWrite) != 0 || ResolveLocalSize() is not null), + "Only expected if L2 is disabled and either L1 is disabled or LocalSize is known."); // set a result from a value we calculated directly CacheItem cacheItem; From ab176d4d8abb38c268b2ad5614dead5f4bd85966 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Thu, 4 Jun 2026 16:32:24 +0200 Subject: [PATCH 06/16] Formatting --- .../Internal/DefaultHybridCache.StampedeStateT.cs | 1 + .../Internal/DefaultHybridCache.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index 394cb6fe0e6..b7b8e1f6627 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -378,6 +378,7 @@ private async Task BackgroundFetchAsync() // serialized byte length as the size). When the caller or factory has supplied // LocalSize via the entry options the size is already known, so this last // case does not apply. + CacheItem cacheItem = CacheItem; long? knownLocalSize = ResolveLocalSize(); bool skipSerialize = cacheItem is ImmutableCacheItem diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs index 382b20995fd..33541b2fad1 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs @@ -203,12 +203,6 @@ public override ValueTask GetOrCreateAsync(string key, TState stat return stampede.JoinAsync(_logger, cancellationToken); } - public override ValueTask RemoveAsync(string key, CancellationToken token = default) - { - _localCache.Remove(key); - return _backendCache is null ? default : new(_backendCache.RemoveAsync(key, token)); - } - public override ValueTask GetOrCreateAsync(string key, TState state, Func> factory, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) @@ -278,6 +272,12 @@ public override ValueTask GetOrCreateAsync(string key, TState stat return stampede.JoinAsync(_logger, cancellationToken); } + public override ValueTask RemoveAsync(string key, CancellationToken token = default) + { + _localCache.Remove(key); + return _backendCache is null ? default : new(_backendCache.RemoveAsync(key, token)); + } + private static ValueTask RunWithoutCacheAsync(HybridCacheEntryFlags flags, TState state, Func> factory, HybridCacheEntryOptions? options, From b4f15c3ed120cbed95078cc98bb68c6d5887c71b Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Thu, 11 Jun 2026 13:55:31 +0200 Subject: [PATCH 07/16] Make sure hard flags are not replaced by the factory --- .../DefaultHybridCache.StampedeStateT.cs | 24 ++-- .../PayloadTests.cs | 114 ++++++++++++++++++ 2 files changed, 130 insertions(+), 8 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index b7b8e1f6627..9c68da66b48 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -200,12 +200,18 @@ private async Task BackgroundFetchAsync() try { HybridCacheEntryFlags activeFlags = Key.Flags; + + // Track write-side flags that are required regardless of + // anything the user-supplied options or the factory say. + HybridCacheEntryFlags mandatoryWriteSideFlags = Cache._hardFlags & WriteSideFlags; + if ((activeFlags & HybridCacheEntryFlags.DisableDistributedCache) != HybridCacheEntryFlags.DisableDistributedCache) { // in order to use distributed cache, the tags and keys must be valid unicode, to avoid security complications if (!ValidateUnicodeCorrectness(Cache._logger, Key.Key, CacheItem.Tags)) { activeFlags |= HybridCacheEntryFlags.DisableDistributedCache; + mandatoryWriteSideFlags |= HybridCacheEntryFlags.DisableDistributedCacheWrite; } } @@ -336,7 +342,7 @@ private async Task BackgroundFetchAsync() // as a fast-path skip for the common case where the factory didn't touch them. if (_underlyingWithOptions is not null && _options!.Revision != _factoryOptionsRevision) { - ApplyFactoryOptions(_options, ref activeFlags); + ApplyFactoryOptions(_options, mandatoryWriteSideFlags, ref activeFlags); } // check whether we're going to hit a timing problem with tag invalidation @@ -621,16 +627,18 @@ private void SetResult(CacheItem value, HybridCacheEntryFlags activeFlags, Ti /// /// Applies factory mutations to the active flags after the factory callback has executed. /// Only write-side flags are honored; read-side flags are ignored (reads already happened). - /// Expiration / LocalCacheExpiration / LocalSize mutations need no action here because the - /// factory wrote directly to , which is read by SetL1 / SetL2Async / ResolveLocalSize. + /// The factory's write-side flags fully replace the user-supplied write-side flags, but + /// are always preserved. Expiration / LocalCacheExpiration / LocalSize mutations need no + /// action here because the factory wrote directly to , which is read + /// by SetL1 / SetL2Async / ResolveLocalSize. /// - private static void ApplyFactoryOptions(HybridCacheEntryOptions factoryOptions, ref HybridCacheEntryFlags activeFlags) + private static void ApplyFactoryOptions( + HybridCacheEntryOptions factoryOptions, + HybridCacheEntryFlags mandatoryWriteSideFlags, + ref HybridCacheEntryFlags activeFlags) { - // Replace write-side bits with whatever the factory left behind; preserve everything else - // (including read-side flags already acted on, plus any constraints we OR'd in earlier such - // as DisableDistributedCache from validation). HybridCacheEntryFlags factoryFlags = factoryOptions.Flags ?? HybridCacheEntryFlags.None; - activeFlags = (activeFlags & ~WriteSideFlags) | (factoryFlags & WriteSideFlags); + activeFlags = (activeFlags & ~WriteSideFlags) | (factoryFlags & WriteSideFlags) | mandatoryWriteSideFlags; } } diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs index 256f8298b0d..8e9b0f3edfd 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs @@ -381,6 +381,120 @@ public async Task MalformedTagDetected() collector.AssertErrors([Log.IdTagInvalidUnicode]); } + public enum FactoryMutation + { + MutateNonFlags, + SetFlagsToNone, + SetFlagsReadSideOnly, + CallerDisabledL2Write_FactoryClears, + } + + [Theory] + [InlineData(FactoryMutation.MutateNonFlags)] + [InlineData(FactoryMutation.SetFlagsToNone)] + [InlineData(FactoryMutation.SetFlagsReadSideOnly)] + [InlineData(FactoryMutation.CallerDisabledL2Write_FactoryClears)] + public async Task MalformedKey_DoesNotWriteToL2_EvenWhenFactoryMutatesOptions(FactoryMutation mutation) + { + // When the key fails unicode validation, BackgroundFetchAsync makes sure that the (corrupted) key/tags are + // not persisted to L2. ApplyFactoryOptions must preserve that safeguard. + using var collector = new LogCollector(); + MemoryDistributedCache? localCache = null; + using var provider = GetDefaultCache(out var cache, config => + { + localCache = new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions())); + config.AddSingleton(new LoggingCache(log, localCache)); + config.AddLogging(options => + { + options.ClearProviders(); + options.AddProvider(collector); + }); + }); + + Assert.NotNull(localCache); + + string key = "my\uD801\uD802key"; // malformed unicode + string[] tags = ["mytag"]; + + HybridCacheEntryFlags callerFlags = mutation == FactoryMutation.CallerDisabledL2Write_FactoryClears + ? HybridCacheEntryFlags.DisableDistributedCacheWrite + : HybridCacheEntryFlags.None; + + _ = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + switch (mutation) + { + case FactoryMutation.MutateNonFlags: + entryOptions.LocalSize = 1234; + break; + case FactoryMutation.SetFlagsToNone: + entryOptions.Flags = HybridCacheEntryFlags.None; + break; + case FactoryMutation.SetFlagsReadSideOnly: + entryOptions.Flags = HybridCacheEntryFlags.DisableLocalCacheRead; + break; + case FactoryMutation.CallerDisabledL2Write_FactoryClears: + entryOptions.Flags = HybridCacheEntryFlags.None; + break; + } + + return new ValueTask(Guid.NewGuid()); + }, + options: new HybridCacheEntryOptions { Expiration = TimeSpan.FromMinutes(1), Flags = callerFlags }, + tags: tags); + + // Wait until the unicode-validation log fires (synchronous, inside BackgroundFetchAsync, + // just before the L2 write decision); then give the background work a brief moment + // to flush any erroneous L2 SetAsync, mirroring the pattern in RedisTests. + await collector.WaitForLogsAsync([Log.IdKeyInvalidUnicode], TimeSpan.FromSeconds(5)); + await Task.Delay(500); + + collector.WriteTo(log); + collector.AssertErrors([Log.IdKeyInvalidUnicode]); + + // The corrupted key must NOT have been persisted to L2. (Note: unrelated + // tag-invalidation reads against valid tag keys may still appear in the backend.) + Assert.Null(localCache.Get(key)); + } + + [Fact] + public async Task FactoryCanReEnableL2Write_ThatCallerDisabled() + { + // Positive counterpart to the malformed-key tests: when the key is valid and the + // caller passed DisableDistributedCacheWrite, the factory is allowed to clear that + // bit on its options and the value should then be persisted to L2. + MemoryDistributedCache? localCache = null; + using var provider = GetDefaultCache(out var cache, config => + { + localCache = new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions())); + config.AddSingleton(new LoggingCache(log, localCache)); + }); + + Assert.NotNull(localCache); + + string key = "my-valid-key"; + + _ = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + entryOptions.Flags = HybridCacheEntryFlags.None; + return new ValueTask(Guid.NewGuid()); + }, + options: new HybridCacheEntryOptions + { + Expiration = TimeSpan.FromMinutes(1), + Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite, + }); + + // Give the background L2 write a chance to complete. + await Task.Delay(500); + + Assert.NotNull(localCache.Get(key)); + } + [Theory] [InlineData("tag1,tag2", 2)] [InlineData("tag1,tag2,tag3", 3)] From 7f3ca9364a0c0adea84569e45349f469cf0e2cc9 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Thu, 11 Jun 2026 16:37:53 +0200 Subject: [PATCH 08/16] Added more tests --- .../FactoryOptionsTests.cs | 401 ++++++++++++++++++ .../PayloadTests.cs | 36 -- 2 files changed, 401 insertions(+), 36 deletions(-) create mode 100644 test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs new file mode 100644 index 00000000000..1d8d5a5853b --- /dev/null +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs @@ -0,0 +1,401 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.Caching.Hybrid.Internal; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Internal; +using Xunit.Abstractions; +using static Microsoft.Extensions.Caching.Hybrid.Tests.DistributedCacheTests; +using static Microsoft.Extensions.Caching.Hybrid.Tests.L2Tests; + +namespace Microsoft.Extensions.Caching.Hybrid.Tests; + +// Covers mutations the factory makes to the HybridCacheEntryOptions it is handed — +// both Flags (replace semantics, subject to runtime-mandated floor) and non-Flags +// properties (Expiration / LocalCacheExpiration / LocalSize) that the factory writes +// directly to the options instance and which are read by SetL1 / SetL2Async / ResolveLocalSize. +public class FactoryOptionsTests(ITestOutputHelper log) : IClassFixture +{ + private static (DefaultHybridCache cache, MemoryDistributedCache localCache, ServiceProvider provider) BuildCacheWithL2(ITestOutputHelper log) + { + var localCache = new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions())); + var services = new ServiceCollection(); + services.AddSingleton(new LoggingCache(log, localCache)); + services.AddHybridCache(); + var provider = services.BuildServiceProvider(); + var cache = Assert.IsType(provider.GetRequiredService()); + return (cache, localCache, provider); + } + + [Fact] + public async Task FactoryCanReEnableL2Write_ThatCallerDisabled() + { + // Caller disabled L2 writes; factory clears the flag — value must be persisted to L2. + var (cache, localCache, provider) = BuildCacheWithL2(log); + using (provider) + { + string key = nameof(FactoryCanReEnableL2Write_ThatCallerDisabled); + + _ = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + entryOptions.Flags = HybridCacheEntryFlags.None; + return new ValueTask(Guid.NewGuid()); + }, + options: new HybridCacheEntryOptions + { + Expiration = TimeSpan.FromMinutes(1), + Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite, + }); + + await Task.Delay(500); + Assert.NotNull(localCache.Get(key)); + } + } + + [Fact] + public async Task FactoryCanDisableL2Write_ThatCallerEnabled() + { + // Symmetric tightening: caller allowed L2 writes (None), factory disables them. + // Replace-semantics in ApplyFactoryOptions must make the factory's restriction stick. + var (cache, localCache, provider) = BuildCacheWithL2(log); + using (provider) + { + string key = nameof(FactoryCanDisableL2Write_ThatCallerEnabled); + + _ = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + entryOptions.Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite; + return new ValueTask(Guid.NewGuid()); + }, + options: new HybridCacheEntryOptions + { + Expiration = TimeSpan.FromMinutes(1), + Flags = HybridCacheEntryFlags.None, + }); + + await Task.Delay(500); + Assert.Null(localCache.Get(key)); + } + } + + [Fact] + public async Task FactoryCanEnableL1Write_ThatCallerDisabled() + { + // L1 counterpart of FactoryCanReEnableL2Write: caller passed DisableLocalCacheWrite, + // factory clears it. If the override sticks, a subsequent read returns the same value + // from L1 without re-invoking the factory. + var services = new ServiceCollection(); + services.AddHybridCache(); + using var provider = services.BuildServiceProvider(); + var cache = provider.GetRequiredService(); + + string key = nameof(FactoryCanEnableL1Write_ThatCallerDisabled); + int factoryCalls = 0; + + var first = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + Interlocked.Increment(ref factoryCalls); + entryOptions.Flags = HybridCacheEntryFlags.None; + return new ValueTask(Guid.NewGuid()); + }, + options: new HybridCacheEntryOptions + { + Expiration = TimeSpan.FromMinutes(1), + Flags = HybridCacheEntryFlags.DisableLocalCacheWrite, + }); + + Assert.Equal(1, factoryCalls); + + // Second call should be served from L1 — same Guid, factory not invoked again. + var second = await cache.GetOrCreateAsync(key, _ => new(Guid.NewGuid())); + Assert.Equal(first, second); + Assert.Equal(1, factoryCalls); + } + + [Fact] + public async Task FactoryExpirationMutation_PropagatesToL2() + { + // Factory mutates Expiration; the L2 backend must receive DistributedCacheEntryOptions + // whose AbsoluteExpirationRelativeToNow matches the factory-set value. + var captured = new CapturingCache(new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions()))); + var services = new ServiceCollection(); + services.AddSingleton(captured); + services.AddHybridCache(); + using var provider = services.BuildServiceProvider(); + var cache = provider.GetRequiredService(); + + var factoryExpiration = TimeSpan.FromMinutes(7); + _ = await cache.GetOrCreateAsync( + nameof(FactoryExpirationMutation_PropagatesToL2), + (entryOptions, _) => + { + entryOptions.Expiration = factoryExpiration; + return new ValueTask(Guid.NewGuid()); + }, + options: new HybridCacheEntryOptions { Expiration = TimeSpan.FromMinutes(1) }); + + await Task.Delay(500); + Assert.Equal(factoryExpiration, captured.LastSetOptions?.AbsoluteExpirationRelativeToNow); + } + + [Fact] + public async Task FactoryLocalCacheExpirationMutation_ShortensL1Only() + { + // Factory sets LocalCacheExpiration tighter than Expiration. After the L1 entry expires + // but before the overall entry does, the next call must re-fetch from L2 (factory not + // invoked again), proving the factory-set L1 expiration was honored. + var clock = new FakeTime(); + using var l1 = new MemoryCache(new MemoryCacheOptions { Clock = clock }); + var l2 = new LoggingCache(log, new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions { Clock = clock }))); + + var services = new ServiceCollection(); + services.AddSingleton(clock); + services.AddSingleton(clock); + services.AddSingleton(l1); + services.AddSingleton(l2); + services.AddHybridCache(); + using var provider = services.BuildServiceProvider(); + var cache = provider.GetRequiredService(); + + string key = nameof(FactoryLocalCacheExpirationMutation_ShortensL1Only); + int factoryCalls = 0; + Func> factory = (entryOptions, _) => + { + Interlocked.Increment(ref factoryCalls); + entryOptions.LocalCacheExpiration = TimeSpan.FromSeconds(30); + return new ValueTask(Guid.NewGuid()); + }; + + var options = new HybridCacheEntryOptions { Expiration = TimeSpan.FromMinutes(5) }; + + var first = await cache.GetOrCreateAsync(key, factory, options); + Assert.Equal(1, factoryCalls); + + // Past factory-set L1 expiration (30s) but well before overall expiration (5m). + clock.Add(TimeSpan.FromSeconds(45)); + + var second = await cache.GetOrCreateAsync(key, factory, options); + + // Same value (came from L2 round-trip), factory not invoked again. + Assert.Equal(first, second); + Assert.Equal(1, factoryCalls); + } + + [Fact] + public async Task FactoryLocalSizeMutation_HonoredForL1SizeAccounting() + { + // Tight L1 SizeLimit + a payload large enough to exceed it. Without the override the + // entry would be evicted from L1; the factory sets LocalSize = 1 to make the entry fit. + // Verify by issuing a second call: if L1 retained the value, the factory is not + // re-invoked and the same Guid is returned. + var services = new ServiceCollection(); + services.AddMemoryCache(options => options.SizeLimit = 5); + services.AddHybridCache(); + using var provider = services.BuildServiceProvider(); + var cache = provider.GetRequiredService(); + + string key = nameof(FactoryLocalSizeMutation_HonoredForL1SizeAccounting); + int factoryCalls = 0; + + // Use a string payload large enough that its serialized size would otherwise exceed + // the L1 SizeLimit (the default L1 size for a string is its byte length). + string payload = new('x', 256); + + var first = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + Interlocked.Increment(ref factoryCalls); + entryOptions.LocalSize = 1; + return new ValueTask(payload); + }, + options: new HybridCacheEntryOptions + { + Expiration = TimeSpan.FromMinutes(1), + Flags = HybridCacheEntryFlags.DisableDistributedCache, // no L2; force L1-only path + }); + + Assert.Equal(1, factoryCalls); + + // Second call: served from L1 because the override kept the entry under SizeLimit. + var second = await cache.GetOrCreateAsync(key, _ => new(Guid.NewGuid().ToString())); + Assert.Equal(first, second); + Assert.Equal(1, factoryCalls); + } + + [Fact] + public async Task FactoryMutations_DoNotLeakToCallerOptionsInstance() + { + // The implementation passes a clone (or fresh instance) of the caller's options to the + // factory so that any mutations the factory performs do not bleed back into the caller's + // shared instance. A caller that reuses the same options across many calls must see the + // exact values it constructed. + var services = new ServiceCollection(); + services.AddHybridCache(); + using var provider = services.BuildServiceProvider(); + var cache = provider.GetRequiredService(); + + var callerOptions = new HybridCacheEntryOptions + { + Expiration = TimeSpan.FromMinutes(1), + LocalCacheExpiration = TimeSpan.FromSeconds(30), + LocalSize = 100, + Flags = HybridCacheEntryFlags.None, + }; + + // Snapshot before + var origExpiration = callerOptions.Expiration; + var origLocalCacheExpiration = callerOptions.LocalCacheExpiration; + var origLocalSize = callerOptions.LocalSize; + var origFlags = callerOptions.Flags; + + _ = await cache.GetOrCreateAsync( + nameof(FactoryMutations_DoNotLeakToCallerOptionsInstance), + (entryOptions, _) => + { + // Aggressively mutate everything; none of this should leak. + Assert.NotSame(callerOptions, entryOptions); + entryOptions.Expiration = TimeSpan.FromHours(99); + entryOptions.LocalCacheExpiration = TimeSpan.FromHours(99); + entryOptions.LocalSize = 9_999_999; + entryOptions.Flags = HybridCacheEntryFlags.DisableDistributedCache | HybridCacheEntryFlags.DisableLocalCache; + return new ValueTask(Guid.NewGuid()); + }, + options: callerOptions); + + Assert.Equal(origExpiration, callerOptions.Expiration); + Assert.Equal(origLocalCacheExpiration, callerOptions.LocalCacheExpiration); + Assert.Equal(origLocalSize, callerOptions.LocalSize); + Assert.Equal(origFlags, callerOptions.Flags); + } + + [Fact] + public async Task FactoryReceivesUsableOptions_WhenCallerPassedNull() + { + // The options-aware overload must hand the factory a real, mutable instance even when + // the caller did not supply one — otherwise the factory cannot set entryOptions.Flags + // etc. without a NullReferenceException, and the documented "mutate-in-factory" API + // shape would be unusable in the common case. We observe both that the options object + // is non-null and mutable, and that the mutation actually takes effect (no L2 write). + var (cache, localCache, provider) = BuildCacheWithL2(log); + using (provider) + { + string key = nameof(FactoryReceivesUsableOptions_WhenCallerPassedNull); + int factoryCalls = 0; + + _ = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + Interlocked.Increment(ref factoryCalls); + Assert.NotNull(entryOptions); + entryOptions.Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite; + return new ValueTask(Guid.NewGuid()); + }); + + Assert.Equal(1, factoryCalls); + + // The factory's mutation must have taken effect — no value written to L2. + await Task.Delay(500); + Assert.Null(localCache.Get(key)); + } + } + + [Fact] + public async Task FactoryLocalSize_PersistedInL2_AndReappliedOnL2Reload() + { + // End-to-end exercise of commit 15d649097d: the factory-set LocalSize must be persisted + // into the L2 payload so a *different* cache instance reading from the shared L2 still + // gets the size override applied to its L1 entry. + // + // Setup: + // Cache A: unlimited L1 SizeLimit; factory sets LocalSize=1, payload is 256 bytes. + // Cache B: shares L2 with A, has SizeLimit=5. On its first read it must fetch from L2, + // and the persisted LocalSize=1 must be reapplied to its L1 entry so the (256-byte) + // value fits. A second read on B then comes from L1 — observable via the LoggingCache + // L2 op count not increasing. + var sharedL2 = new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions())); + string key = nameof(FactoryLocalSize_PersistedInL2_AndReappliedOnL2Reload); + string payload = new('x', 256); + + // ---- Cache A: writes with factory-set LocalSize override ---- + var loggingA = new LoggingCache(log, sharedL2); + var servicesA = new ServiceCollection(); + servicesA.AddMemoryCache(); // unlimited + servicesA.AddSingleton(loggingA); + servicesA.AddHybridCache(); + using (var providerA = servicesA.BuildServiceProvider()) + { + var cacheA = providerA.GetRequiredService(); + _ = await cacheA.GetOrCreateAsync( + key, + (entryOptions, _) => + { + entryOptions.LocalSize = 1; + return new ValueTask(payload); + }, + options: new HybridCacheEntryOptions { Expiration = TimeSpan.FromMinutes(5) }); + + // give the background L2 write a chance to land + await Task.Delay(500); + } + + // Confirm the value was actually written to L2 by Cache A. + Assert.NotNull(sharedL2.Get(key)); + + // ---- Cache B: shares L2, has a tight SizeLimit that the raw payload would exceed ---- + var loggingB = new LoggingCache(log, sharedL2); + var servicesB = new ServiceCollection(); + servicesB.AddMemoryCache(options => options.SizeLimit = 5); + servicesB.AddSingleton(loggingB); + servicesB.AddHybridCache(); + using var providerB = servicesB.BuildServiceProvider(); + var cacheB = providerB.GetRequiredService(); + + // First call: L1 miss, fetches from L2; persisted LocalSize override must let it fit in L1. + var firstB = await cacheB.GetOrCreateAsync(key, _ => new(Guid.NewGuid().ToString())); + Assert.Equal(payload, firstB); + int opsAfterFirst = loggingB.OpCount; + + // Second call must hit L1 — no further L2 traffic. If the LocalSize override + // wasn't reapplied, the entry would have been evicted from L1 immediately and + // this call would have to re-read L2. + var secondB = await cacheB.GetOrCreateAsync(key, _ => new(Guid.NewGuid().ToString())); + Assert.Equal(payload, secondB); + Assert.Equal(opsAfterFirst, loggingB.OpCount); + } + + // Test-only IDistributedCache that records the DistributedCacheEntryOptions from the + // most recent Set/SetAsync invocation so tests can assert against it. + private sealed class CapturingCache(IDistributedCache tail) : IDistributedCache + { + public DistributedCacheEntryOptions? LastSetOptions { get; private set; } + + public byte[]? Get(string key) => tail.Get(key); + public Task GetAsync(string key, CancellationToken token = default) => tail.GetAsync(key, token); + public void Refresh(string key) => tail.Refresh(key); + public Task RefreshAsync(string key, CancellationToken token = default) => tail.RefreshAsync(key, token); + public void Remove(string key) => tail.Remove(key); + public Task RemoveAsync(string key, CancellationToken token = default) => tail.RemoveAsync(key, token); + + public void Set(string key, byte[] value, DistributedCacheEntryOptions options) + { + LastSetOptions = options; + tail.Set(key, value, options); + } + + public Task SetAsync(string key, byte[] value, DistributedCacheEntryOptions options, CancellationToken token = default) + { + LastSetOptions = options; + return tail.SetAsync(key, value, options, token); + } + } +} diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs index 8e9b0f3edfd..f000a241b9f 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs @@ -459,42 +459,6 @@ public async Task MalformedKey_DoesNotWriteToL2_EvenWhenFactoryMutatesOptions(Fa Assert.Null(localCache.Get(key)); } - [Fact] - public async Task FactoryCanReEnableL2Write_ThatCallerDisabled() - { - // Positive counterpart to the malformed-key tests: when the key is valid and the - // caller passed DisableDistributedCacheWrite, the factory is allowed to clear that - // bit on its options and the value should then be persisted to L2. - MemoryDistributedCache? localCache = null; - using var provider = GetDefaultCache(out var cache, config => - { - localCache = new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions())); - config.AddSingleton(new LoggingCache(log, localCache)); - }); - - Assert.NotNull(localCache); - - string key = "my-valid-key"; - - _ = await cache.GetOrCreateAsync( - key, - (entryOptions, _) => - { - entryOptions.Flags = HybridCacheEntryFlags.None; - return new ValueTask(Guid.NewGuid()); - }, - options: new HybridCacheEntryOptions - { - Expiration = TimeSpan.FromMinutes(1), - Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite, - }); - - // Give the background L2 write a chance to complete. - await Task.Delay(500); - - Assert.NotNull(localCache.Get(key)); - } - [Theory] [InlineData("tag1,tag2", 2)] [InlineData("tag1,tag2,tag3", 3)] From 2c7d81b4518612d72fdbdbfa8ee06403895af251 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Fri, 12 Jun 2026 13:58:18 +0200 Subject: [PATCH 09/16] More tests --- .../FactoryOptionsTests.cs | 136 ++++++++++++++++-- .../PayloadTests.cs | 28 ++++ 2 files changed, 152 insertions(+), 12 deletions(-) diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs index 1d8d5a5853b..cbb3d7ce467 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Concurrent; +using System.Reflection; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Caching.Hybrid.Internal; using Microsoft.Extensions.Caching.Memory; @@ -18,9 +20,9 @@ namespace Microsoft.Extensions.Caching.Hybrid.Tests; // directly to the options instance and which are read by SetL1 / SetL2Async / ResolveLocalSize. public class FactoryOptionsTests(ITestOutputHelper log) : IClassFixture { - private static (DefaultHybridCache cache, MemoryDistributedCache localCache, ServiceProvider provider) BuildCacheWithL2(ITestOutputHelper log) + private static (DefaultHybridCache cache, CapturingCache localCache, ServiceProvider provider) BuildCacheWithL2(ITestOutputHelper log) { - var localCache = new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions())); + var localCache = new CapturingCache(new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions()))); var services = new ServiceCollection(); services.AddSingleton(new LoggingCache(log, localCache)); services.AddHybridCache(); @@ -29,6 +31,12 @@ private static (DefaultHybridCache cache, MemoryDistributedCache localCache, Ser return (cache, localCache, provider); } + private static Task WaitForBackgroundL2WriteAsync(CapturingCache cache, string key) + { + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + return cache.WaitForWriteAsync(key, cts.Token); + } + [Fact] public async Task FactoryCanReEnableL2Write_ThatCallerDisabled() { @@ -51,7 +59,7 @@ public async Task FactoryCanReEnableL2Write_ThatCallerDisabled() Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite, }); - await Task.Delay(500); + await WaitForBackgroundL2WriteAsync(localCache, key); Assert.NotNull(localCache.Get(key)); } } @@ -142,7 +150,7 @@ public async Task FactoryExpirationMutation_PropagatesToL2() }, options: new HybridCacheEntryOptions { Expiration = TimeSpan.FromMinutes(1) }); - await Task.Delay(500); + await WaitForBackgroundL2WriteAsync(captured, nameof(FactoryExpirationMutation_PropagatesToL2)); Assert.Equal(factoryExpiration, captured.LastSetOptions?.AbsoluteExpirationRelativeToNow); } @@ -327,10 +335,10 @@ public async Task FactoryLocalSize_PersistedInL2_AndReappliedOnL2Reload() string payload = new('x', 256); // ---- Cache A: writes with factory-set LocalSize override ---- - var loggingA = new LoggingCache(log, sharedL2); + var capturingA = new CapturingCache(sharedL2); var servicesA = new ServiceCollection(); servicesA.AddMemoryCache(); // unlimited - servicesA.AddSingleton(loggingA); + servicesA.AddSingleton(capturingA); servicesA.AddHybridCache(); using (var providerA = servicesA.BuildServiceProvider()) { @@ -344,8 +352,7 @@ public async Task FactoryLocalSize_PersistedInL2_AndReappliedOnL2Reload() }, options: new HybridCacheEntryOptions { Expiration = TimeSpan.FromMinutes(5) }); - // give the background L2 write a chance to land - await Task.Delay(500); + await WaitForBackgroundL2WriteAsync(capturingA, key); } // Confirm the value was actually written to L2 by Cache A. @@ -373,12 +380,38 @@ public async Task FactoryLocalSize_PersistedInL2_AndReappliedOnL2Reload() Assert.Equal(opsAfterFirst, loggingB.OpCount); } - // Test-only IDistributedCache that records the DistributedCacheEntryOptions from the - // most recent Set/SetAsync invocation so tests can assert against it. + // Test-only IDistributedCache that wraps another IDistributedCache and adds: + // - LastSetOptions: the DistributedCacheEntryOptions from the most recent Set/SetAsync, + // for tests that assert on the options the HybridCache layer produced. + // - WaitForWriteAsync(key): a Task that completes after the (possibly background) write + // for that key has finished, so tests can wait deterministically instead of sleeping. private sealed class CapturingCache(IDistributedCache tail) : IDistributedCache { + private readonly ConcurrentDictionary> _writes = new(StringComparer.Ordinal); + public DistributedCacheEntryOptions? LastSetOptions { get; private set; } + public Task WaitForWriteAsync(string key, CancellationToken cancellationToken = default) + { + var tcs = SignalFor(key); + if (!cancellationToken.CanBeCanceled) + { + return tcs.Task; + } + + return WaitWithCancellationAsync(tcs.Task, cancellationToken); + + static async Task WaitWithCancellationAsync(Task task, CancellationToken ct) + { + var cancelTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using (ct.Register(static state => ((TaskCompletionSource)state!).TrySetCanceled(), cancelTcs)) + { + var completed = await Task.WhenAny(task, cancelTcs.Task).ConfigureAwait(false); + await completed.ConfigureAwait(false); + } + } + } + public byte[]? Get(string key) => tail.Get(key); public Task GetAsync(string key, CancellationToken token = default) => tail.GetAsync(key, token); public void Refresh(string key) => tail.Refresh(key); @@ -390,12 +423,91 @@ public void Set(string key, byte[] value, DistributedCacheEntryOptions options) { LastSetOptions = options; tail.Set(key, value, options); + _ = SignalFor(key).TrySetResult(true); } - public Task SetAsync(string key, byte[] value, DistributedCacheEntryOptions options, CancellationToken token = default) + public async Task SetAsync(string key, byte[] value, DistributedCacheEntryOptions options, CancellationToken token = default) { LastSetOptions = options; - return tail.SetAsync(key, value, options, token); + await tail.SetAsync(key, value, options, token).ConfigureAwait(false); + _ = SignalFor(key).TrySetResult(true); + } + + private TaskCompletionSource SignalFor(string key) + => _writes.GetOrAdd(key, _ => new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)); + } + + // Guards against silent data loss when HybridCacheEntryOptions (owned by dotnet/runtime) + // gains a new property and the down-level branch of CloneOptionsOrNew is not updated. + // Also exercises the UnsafeAccessor path on net8.0+. + [Fact] + public void CloneOptionsOrNew_CopiesEveryPublicWritableProperty() + { + var writableProps = typeof(HybridCacheEntryOptions) + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.CanRead && p.CanWrite) + // Revision is an internal mutation counter used by StampedeStateT to detect + // factory mutations; a clone deliberately starts fresh so the diff is meaningful. + .Where(p => p.Name != "Revision") + .ToArray(); + + Assert.NotEmpty(writableProps); + + var source = new HybridCacheEntryOptions(); + var expected = new Dictionary(writableProps.Length); + foreach (var prop in writableProps) + { + object? value = MakeDistinctiveValue(prop.PropertyType, prop.Name); + prop.SetValue(source, value); + expected[prop.Name] = value; + } + + var clone = DefaultHybridCache.CloneOptionsOrNew(source); + + Assert.NotSame(source, clone); + foreach (var prop in writableProps) + { + Assert.Equal(expected[prop.Name], prop.GetValue(clone)); + } + } + + private static object MakeDistinctiveValue(Type t, string propName) + { + var underlying = Nullable.GetUnderlyingType(t) ?? t; + + // Per-property unique values so cross-wired assignments + // (e.g. Expiration <-> LocalCacheExpiration) are also caught. + if (underlying == typeof(TimeSpan)) + { + return TimeSpan.FromSeconds((StableHash(propName) % 3600) + 1); + } + + if (underlying == typeof(long)) + { + return (long)StableHash(propName) + 1; + } + + if (underlying.IsEnum) + { + var nonDefault = Enum.GetValues(underlying).Cast() + .FirstOrDefault(v => !v.Equals(Activator.CreateInstance(underlying))); + return nonDefault ?? Activator.CreateInstance(underlying)!; } + + throw new NotSupportedException( + $"HybridCacheEntryOptions has a new property '{propName}' of type {underlying.FullName}. " + + $"Add a distinctive value generator here AND update DefaultHybridCache.CloneOptionsOrNew."); + } + + private static int StableHash(string s) + { + // FNV-ish stable hash; avoids dependence on string.GetHashCode randomization. + int h = 17; + foreach (char c in s) + { + h = unchecked((h * 31) + c); + } + + return Math.Abs(h); } } diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs index f000a241b9f..cdb34abcf87 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs @@ -119,6 +119,34 @@ public void RoundTrip_LocalCacheSize_NegativeSentinelIsNormalizedToAbsent(long l ArrayPool.Shared.Return(oversized); } + // Guards the v1 wire format. This test pins a literal v1 byte sequence and asserts the reader + // still accepts it. + // + // The bytes below were produced by `HybridCachePayload.Write` with: + // key="frozen-key", tags=Empty, payload=[0x01..0x08], + // creationTime = 638000000000000000 ticks (~2023-01-30 UTC), + // duration = TimeSpan.FromDays(36500) (100 years headroom so the payload stays valid). + [Fact] + public void V1_FrozenBytes_StillReadable() + { + using var provider = GetDefaultCache(out var cache); + + byte[] frozen = Convert.FromBase64String( + "AwGPlwAAs6aeodoIAAiAgIztsrqCOAAKZnJvemVuLWtleQECAwQFBgcIAwE="); + + var result = HybridCachePayload.TryParse( + new(frozen), "frozen-key", TagSet.Empty, cache, + out var payload, out var remaining, out var flags, out _, out var pendingTags, + out long? parsedSize, out _); + + Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); + Assert.True(payload.AsSpan().SequenceEqual([ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08 ])); + Assert.True(pendingTags.IsEmpty); + Assert.Null(parsedSize); + Assert.Equal(HybridCachePayload.PayloadFlags.None, flags & HybridCachePayload.PayloadFlags.HasLocalSize); + Assert.True(remaining > TimeSpan.Zero, "v1 frozen entry should not be expired (100-year duration)."); + } + [Fact] public void RoundTrip_SelfExpiration() { From 75be9330bcc4b95b65c30f674485c92707eca259 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Fri, 12 Jun 2026 16:13:15 +0200 Subject: [PATCH 10/16] Refactoring --- .../DefaultHybridCache.StampedeStateT.cs | 40 ++-- .../Internal/DefaultHybridCache.cs | 188 ++++++++---------- 2 files changed, 103 insertions(+), 125 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index 9c68da66b48..2acb5f72ec4 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -62,50 +62,48 @@ public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions? options) + public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions options) { - Debug.Assert(_underlying is null, "should not already have factory field"); + Debug.Assert(_underlyingWithOptions is null, "should not already have factory field"); Debug.Assert(underlying is not null, "factory argument should be meaningful"); // initialize the callback state _state = state; - _underlying = underlying; + _underlyingWithOptions = underlying; _options = options; + _factoryOptionsRevision = _options.Revision; - return BackgroundFetchAsync(); +#if NETCOREAPP3_0_OR_GREATER + ThreadPool.UnsafeQueueUserWorkItem(this, false); +#else + ThreadPool.UnsafeQueueUserWorkItem(SharedWaitCallback, this); +#endif } - public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions? options) + [SuppressMessage("Resilience", "EA0014:The async method doesn't support cancellation", Justification = "Cancellation is handled separately via SharedToken")] + public Task ExecuteDirectAsync(in TState state, Func> underlying, HybridCacheEntryOptions? options) { - Debug.Assert(_underlyingWithOptions is null, "should not already have factory field"); + Debug.Assert(_underlying is null, "should not already have factory field"); Debug.Assert(underlying is not null, "factory argument should be meaningful"); - // initialize the callback state; we always pass a clone (or fresh instance) to the factory so - // it can mutate the options without affecting the caller's instance, and so we can detect - // mutations via Revision. + // initialize the callback state _state = state; - _underlyingWithOptions = underlying; - _options = CloneOptionsOrNew(options); - _factoryOptionsRevision = _options.Revision; + _underlying = underlying; + _options = options; -#if NETCOREAPP3_0_OR_GREATER - ThreadPool.UnsafeQueueUserWorkItem(this, false); -#else - ThreadPool.UnsafeQueueUserWorkItem(SharedWaitCallback, this); -#endif + return BackgroundFetchAsync(); } [SuppressMessage("Resilience", "EA0014:The async method doesn't support cancellation", Justification = "Cancellation is handled separately via SharedToken")] - public Task ExecuteDirectAsync(in TState state, Func> underlying, HybridCacheEntryOptions? options) + public Task ExecuteDirectAsync(in TState state, Func> underlying, HybridCacheEntryOptions options) { Debug.Assert(_underlyingWithOptions is null, "should not already have factory field"); Debug.Assert(underlying is not null, "factory argument should be meaningful"); - // initialize the callback state (see QueueUserWorkItem above for cloning rationale) + // initialize the callback state _state = state; _underlyingWithOptions = underlying; - _options = CloneOptionsOrNew(options); + _options = options; _factoryOptionsRevision = _options.Revision; return BackgroundFetchAsync(); diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs index 33541b2fad1..82749a88a09 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs @@ -138,86 +138,101 @@ public DefaultHybridCache(HybridCacheOptions options, IServiceProvider services) public override ValueTask GetOrCreateAsync(string key, TState state, Func> underlyingDataCallback, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) { - bool canBeCanceled = cancellationToken.CanBeCanceled; - if (canBeCanceled) + GetOrCreateOutcome outcome = TryBeginGetOrCreate(key, options, tags, cancellationToken, + out HybridCacheEntryFlags flags, out bool canBeCanceled, out StampedeState? stampede, out ValueTask result); + + switch (outcome) { - cancellationToken.ThrowIfCancellationRequested(); + case GetOrCreateOutcome.NoCache: + return (flags & HybridCacheEntryFlags.DisableUnderlyingData) == 0 + ? underlyingDataCallback(state, cancellationToken) : default; + case GetOrCreateOutcome.L1Hit: + case GetOrCreateOutcome.JoinedStampede: + return result; } - HybridCacheEntryFlags flags = GetEffectiveFlags(options); - if (!ValidateKey(key)) + // GetOrCreateOutcome.NewStampede: we own this stampede and must dispatch the factory + if (canBeCanceled) { - // we can't use cache, but we can still provide the data - return RunWithoutCacheAsync(flags, state, underlyingDataCallback, cancellationToken); + // *we* might cancel, but someone else might be depending on the result; start the + // work independently, then join the outcome + stampede!.QueueUserWorkItem(in state, underlyingDataCallback, options); + return stampede.JoinAsync(_logger, cancellationToken); } - bool eventSourceEnabled = HybridCacheEventSource.Log.IsEnabled(); + // we're going to run to completion; no need to get complicated + _ = stampede!.ExecuteDirectAsync(in state, underlyingDataCallback, options); // this larger task includes L2 write etc - if ((flags & HybridCacheEntryFlags.DisableLocalCacheRead) == 0) - { - if (TryGetExisting(key, out CacheItem? typed) - && typed.TryGetValue(_logger, out T? value)) - { - // short-circuit - if (eventSourceEnabled) - { - HybridCacheEventSource.Log.LocalCacheHit(); - } + return stampede.UnwrapReservedAsync(_logger); + } - return new(value); - } - else - { - if (eventSourceEnabled) - { - HybridCacheEventSource.Log.LocalCacheMiss(); - } - } - } + public override ValueTask GetOrCreateAsync(string key, TState state, + Func> factory, + HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) + { + GetOrCreateOutcome outcome = TryBeginGetOrCreate(key, options, tags, cancellationToken, + out HybridCacheEntryFlags flags, out bool canBeCanceled, out StampedeState? stampede, out ValueTask result); - if (GetOrCreateStampedeState(key, flags, out StampedeState? stampede, canBeCanceled, tags)) + // We always pass a clone (or fresh instance) to the factory so + // it can mutate the options without affecting the caller's instance. + // In the NoCache case, there's no cache to honor the mutations against, but the factory may rely on + // being able to mutate the parameter without surprising the caller. + + switch (outcome) { - // new query; we're responsible for making it happen - if (canBeCanceled) - { - // *we* might cancel, but someone else might be depending on the result; start the - // work independently, then we'll with join the outcome - stampede.QueueUserWorkItem(in state, underlyingDataCallback, options); - } - else - { - // we're going to run to completion; no need to get complicated - _ = stampede.ExecuteDirectAsync(in state, underlyingDataCallback, options); // this larger task includes L2 write etc - return stampede.UnwrapReservedAsync(_logger); - } + case GetOrCreateOutcome.NoCache: + return (flags & HybridCacheEntryFlags.DisableUnderlyingData) == 0 + ? factory(state, CloneOptionsOrNew(options), cancellationToken) : default; + case GetOrCreateOutcome.L1Hit: + case GetOrCreateOutcome.JoinedStampede: + return result; } - else + + options = CloneOptionsOrNew(options); + + if (canBeCanceled) { - // pre-existing query - if (eventSourceEnabled) - { - HybridCacheEventSource.Log.StampedeJoin(); - } + stampede!.QueueUserWorkItem(in state, factory, options); + return stampede.JoinAsync(_logger, cancellationToken); } - return stampede.JoinAsync(_logger, cancellationToken); + _ = stampede!.ExecuteDirectAsync(in state, factory, options); + + return stampede.UnwrapReservedAsync(_logger); } - public override ValueTask GetOrCreateAsync(string key, TState state, - Func> factory, - HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) + private enum GetOrCreateOutcome + { + NoCache, + L1Hit, + JoinedStampede, + NewStampede + } + + // Performs the shared pre-factory work for both GetOrCreateAsync overloads. + private GetOrCreateOutcome TryBeginGetOrCreate( + string key, + HybridCacheEntryOptions? options, + IEnumerable? tags, + CancellationToken cancellationToken, + out HybridCacheEntryFlags flags, + out bool canBeCanceled, + out StampedeState? stampede, + out ValueTask result) { - bool canBeCanceled = cancellationToken.CanBeCanceled; + canBeCanceled = cancellationToken.CanBeCanceled; if (canBeCanceled) { cancellationToken.ThrowIfCancellationRequested(); } - HybridCacheEntryFlags flags = GetEffectiveFlags(options); + flags = GetEffectiveFlags(options); + stampede = null; + result = default; + if (!ValidateKey(key)) { - // we can't use cache, but we can still provide the data - return RunWithoutCacheAsync(flags, state, factory, options, cancellationToken); + return GetOrCreateOutcome.NoCache; } bool eventSourceEnabled = HybridCacheEventSource.Log.IsEnabled(); @@ -227,49 +242,34 @@ public override ValueTask GetOrCreateAsync(string key, TState stat if (TryGetExisting(key, out CacheItem? typed) && typed.TryGetValue(_logger, out T? value)) { - // short-circuit if (eventSourceEnabled) { HybridCacheEventSource.Log.LocalCacheHit(); } - return new(value); + result = new(value); + return GetOrCreateOutcome.L1Hit; } - else + + if (eventSourceEnabled) { - if (eventSourceEnabled) - { - HybridCacheEventSource.Log.LocalCacheMiss(); - } + HybridCacheEventSource.Log.LocalCacheMiss(); } } - if (GetOrCreateStampedeState(key, flags, out StampedeState? stampede, canBeCanceled, tags)) + if (GetOrCreateStampedeState(key, flags, out stampede, canBeCanceled, tags)) { - // new query; we're responsible for making it happen - if (canBeCanceled) - { - // *we* might cancel, but someone else might be depending on the result; start the - // work independently, then we'll with join the outcome - stampede.QueueUserWorkItem(in state, factory, options); - } - else - { - // we're going to run to completion; no need to get complicated - _ = stampede.ExecuteDirectAsync(in state, factory, options); // this larger task includes L2 write etc - return stampede.UnwrapReservedAsync(_logger); - } + return GetOrCreateOutcome.NewStampede; } - else + + // joined a pre-existing stampede + if (eventSourceEnabled) { - // pre-existing query - if (eventSourceEnabled) - { - HybridCacheEventSource.Log.StampedeJoin(); - } + HybridCacheEventSource.Log.StampedeJoin(); } - return stampede.JoinAsync(_logger, cancellationToken); + result = stampede.JoinAsync(_logger, cancellationToken); + return GetOrCreateOutcome.JoinedStampede; } public override ValueTask RemoveAsync(string key, CancellationToken token = default) @@ -278,18 +278,6 @@ public override ValueTask RemoveAsync(string key, CancellationToken token = defa return _backendCache is null ? default : new(_backendCache.RemoveAsync(key, token)); } - private static ValueTask RunWithoutCacheAsync(HybridCacheEntryFlags flags, TState state, - Func> factory, - HybridCacheEntryOptions? options, - CancellationToken cancellationToken) - { - // pass a clone (or fresh instance) so the caller's options are never mutated; - // there's no cache to honor the mutations against, but the factory may rely on - // being able to mutate the parameter without surprising the caller. - return (flags & HybridCacheEntryFlags.DisableUnderlyingData) == 0 - ? factory(state, CloneOptionsOrNew(options), cancellationToken) : default; - } - internal static HybridCacheEntryOptions CloneOptionsOrNew(HybridCacheEntryOptions? options) { if (options is null) @@ -333,14 +321,6 @@ internal TimeSpan GetL1AbsoluteExpirationRelativeToNow(HybridCacheEntryOptions? internal HybridCacheEntryFlags GetEffectiveFlags(HybridCacheEntryOptions? options) => (options?.Flags | _hardFlags) ?? _defaultFlags; - private static ValueTask RunWithoutCacheAsync(HybridCacheEntryFlags flags, TState state, - Func> underlyingDataCallback, - CancellationToken cancellationToken) - { - return (flags & HybridCacheEntryFlags.DisableUnderlyingData) == 0 - ? underlyingDataCallback(state, cancellationToken) : default; - } - private static TimeSpan? GetEffectiveLocalCacheExpiration(HybridCacheEntryOptions? options) { // If LocalCacheExpiration is not specified, then use option's Expiration, to keep in sync by default. From 183743d6518d9094eca0257a574b29762182465d Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Fri, 12 Jun 2026 16:21:29 +0200 Subject: [PATCH 11/16] Rename parameters of overrides --- .../DefaultHybridCache.StampedeStateT.cs | 44 +++++++++---------- .../DefaultHybridCache.TagInvalidation.cs | 4 +- .../Internal/DefaultHybridCache.cs | 16 +++---- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index 2acb5f72ec4..0d22553b8ff 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -21,8 +21,8 @@ internal sealed class StampedeState : StampedeState private readonly TaskCompletionSource>? _result; private TState? _state; - private Func>? _underlying; // main data factory - private Func>? _underlyingWithOptions; // options-aware data factory + private Func>? _factory; // main data factory + private Func>? _factoryWithOptions; // options-aware data factory private HybridCacheEntryOptions? _options; private int _factoryOptionsRevision; // initial Revision of the options instance passed to the factory; used to detect mutations private Task? _sharedUnwrap; // allows multiple non-cancellable callers to share a single task (when no defensive copy needed) @@ -45,14 +45,14 @@ public StampedeState(DefaultHybridCache cache, in StampedeKey key, TagSet tags, public override Type Type => typeof(T); - public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions? options) + public void QueueUserWorkItem(in TState state, Func> factory, HybridCacheEntryOptions? options) { - Debug.Assert(_underlying is null, "should not already have factory field"); - Debug.Assert(underlying is not null, "factory argument should be meaningful"); + Debug.Assert(_factory is null, "should not already have factory field"); + Debug.Assert(factory is not null, "factory argument should be meaningful"); // initialize the callback state _state = state; - _underlying = underlying; + _factory = factory; _options = options; #if NETCOREAPP3_0_OR_GREATER @@ -62,14 +62,14 @@ public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions options) + public void QueueUserWorkItem(in TState state, Func> factory, HybridCacheEntryOptions options) { - Debug.Assert(_underlyingWithOptions is null, "should not already have factory field"); - Debug.Assert(underlying is not null, "factory argument should be meaningful"); + Debug.Assert(_factoryWithOptions is null, "should not already have factory field"); + Debug.Assert(factory is not null, "factory argument should be meaningful"); // initialize the callback state _state = state; - _underlyingWithOptions = underlying; + _factoryWithOptions = factory; _options = options; _factoryOptionsRevision = _options.Revision; @@ -81,28 +81,28 @@ public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions? options) + public Task ExecuteDirectAsync(in TState state, Func> factory, HybridCacheEntryOptions? options) { - Debug.Assert(_underlying is null, "should not already have factory field"); - Debug.Assert(underlying is not null, "factory argument should be meaningful"); + Debug.Assert(_factory is null, "should not already have factory field"); + Debug.Assert(factory is not null, "factory argument should be meaningful"); // initialize the callback state _state = state; - _underlying = underlying; + _factory = factory; _options = options; return BackgroundFetchAsync(); } [SuppressMessage("Resilience", "EA0014:The async method doesn't support cancellation", Justification = "Cancellation is handled separately via SharedToken")] - public Task ExecuteDirectAsync(in TState state, Func> underlying, HybridCacheEntryOptions options) + public Task ExecuteDirectAsync(in TState state, Func> factory, HybridCacheEntryOptions options) { - Debug.Assert(_underlyingWithOptions is null, "should not already have factory field"); - Debug.Assert(underlying is not null, "factory argument should be meaningful"); + Debug.Assert(_factoryWithOptions is null, "should not already have factory field"); + Debug.Assert(factory is not null, "factory argument should be meaningful"); // initialize the callback state _state = state; - _underlyingWithOptions = underlying; + _factoryWithOptions = factory; _options = options; _factoryOptionsRevision = _options.Revision; @@ -305,13 +305,13 @@ private async Task BackgroundFetchAsync() HybridCacheEventSource.Log.UnderlyingDataQueryStart(); } - if (_underlyingWithOptions is not null) + if (_factoryWithOptions is not null) { - newValue = await _underlyingWithOptions(_state!, _options!, SharedToken).ConfigureAwait(false); + newValue = await _factoryWithOptions(_state!, _options!, SharedToken).ConfigureAwait(false); } else { - newValue = await _underlying!(_state!, SharedToken).ConfigureAwait(false); + newValue = await _factory!(_state!, SharedToken).ConfigureAwait(false); } if (eventSourceEnabled) @@ -338,7 +338,7 @@ private async Task BackgroundFetchAsync() // honor any mutations the factory made to the options it received; we use Revision // as a fast-path skip for the common case where the factory didn't touch them. - if (_underlyingWithOptions is not null && _options!.Revision != _factoryOptionsRevision) + if (_factoryWithOptions is not null && _options!.Revision != _factoryOptionsRevision) { ApplyFactoryOptions(_options, mandatoryWriteSideFlags, ref activeFlags); } diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.TagInvalidation.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.TagInvalidation.cs index ef5b7f1a01a..92f9aeb4309 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.TagInvalidation.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.TagInvalidation.cs @@ -21,7 +21,7 @@ internal partial class DefaultHybridCache private Task _globalInvalidateTimestamp; - public override ValueTask RemoveByTagAsync(string tag, CancellationToken token = default) + public override ValueTask RemoveByTagAsync(string tag, CancellationToken cancellationToken = default) { if (string.IsNullOrWhiteSpace(tag)) { @@ -30,7 +30,7 @@ public override ValueTask RemoveByTagAsync(string tag, CancellationToken token = long now = CurrentTimestamp(); InvalidateTagLocalCore(tag, now, isNow: true); // isNow to be 100% explicit - return InvalidateL2TagAsync(tag, now, token); + return InvalidateL2TagAsync(tag, now, cancellationToken); } public bool IsValid(CacheItem cacheItem) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs index 82749a88a09..c43b84dec5d 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs @@ -135,7 +135,7 @@ public DefaultHybridCache(HybridCacheOptions options, IServiceProvider services) internal HybridCacheOptions Options => _options; - public override ValueTask GetOrCreateAsync(string key, TState state, Func> underlyingDataCallback, + public override ValueTask GetOrCreateAsync(string key, TState state, Func> factory, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) { GetOrCreateOutcome outcome = TryBeginGetOrCreate(key, options, tags, cancellationToken, @@ -145,7 +145,7 @@ public override ValueTask GetOrCreateAsync(string key, TState stat { case GetOrCreateOutcome.NoCache: return (flags & HybridCacheEntryFlags.DisableUnderlyingData) == 0 - ? underlyingDataCallback(state, cancellationToken) : default; + ? factory(state, cancellationToken) : default; case GetOrCreateOutcome.L1Hit: case GetOrCreateOutcome.JoinedStampede: return result; @@ -156,12 +156,12 @@ public override ValueTask GetOrCreateAsync(string key, TState stat { // *we* might cancel, but someone else might be depending on the result; start the // work independently, then join the outcome - stampede!.QueueUserWorkItem(in state, underlyingDataCallback, options); + stampede!.QueueUserWorkItem(in state, factory, options); return stampede.JoinAsync(_logger, cancellationToken); } // we're going to run to completion; no need to get complicated - _ = stampede!.ExecuteDirectAsync(in state, underlyingDataCallback, options); // this larger task includes L2 write etc + _ = stampede!.ExecuteDirectAsync(in state, factory, options); // this larger task includes L2 write etc return stampede.UnwrapReservedAsync(_logger); } @@ -272,10 +272,10 @@ private GetOrCreateOutcome TryBeginGetOrCreate( return GetOrCreateOutcome.JoinedStampede; } - public override ValueTask RemoveAsync(string key, CancellationToken token = default) + public override ValueTask RemoveAsync(string key, CancellationToken cancellationToken = default) { _localCache.Remove(key); - return _backendCache is null ? default : new(_backendCache.RemoveAsync(key, token)); + return _backendCache is null ? default : new(_backendCache.RemoveAsync(key, cancellationToken)); } internal static HybridCacheEntryOptions CloneOptionsOrNew(HybridCacheEntryOptions? options) @@ -302,12 +302,12 @@ internal static HybridCacheEntryOptions CloneOptionsOrNew(HybridCacheEntryOption #endif } - public override ValueTask SetAsync(string key, T value, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken token = default) + public override ValueTask SetAsync(string key, T value, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) { // since we're forcing a write: disable L1+L2 read; we'll use a direct pass-thru of the value as the callback, to reuse all the code // note also that stampede token is not shared with anyone else HybridCacheEntryFlags flags = GetEffectiveFlags(options) | (HybridCacheEntryFlags.DisableLocalCacheRead | HybridCacheEntryFlags.DisableDistributedCacheRead); - var state = new StampedeState(this, new StampedeKey(key, flags), TagSet.Create(tags), token); + var state = new StampedeState(this, new StampedeKey(key, flags), TagSet.Create(tags), cancellationToken); return new(state.ExecuteDirectAsync(value, static (state, _) => new(state), options)); // note this spans L2 write etc } From 0be7fb5a930a83be11b198e24b1568f7ff43bdf9 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Fri, 12 Jun 2026 17:05:35 +0200 Subject: [PATCH 12/16] Validate options.LocalSize --- .../DefaultHybridCache.MutableCacheItem.cs | 4 +- .../DefaultHybridCache.StampedeStateT.cs | 5 +- .../Internal/DefaultHybridCache.cs | 13 ++ .../FactoryOptionsTests.cs | 176 +++++++++--------- .../FunctionalTests.cs | 27 ++- 5 files changed, 132 insertions(+), 93 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs index 36a5d97dcae..a1230e1a929 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs @@ -13,7 +13,7 @@ internal sealed partial class MutableCacheItem : CacheItem // used to hold private IHybridCacheSerializer? _serializer; private BufferChunk _buffer; private T? _fallbackValue; // only used in the case of serialization failures - private long _localSizeOverride = -1; // -1 means use buffer length + private long? _localSizeOverride; public MutableCacheItem(long creationTimestamp, TagSet tags) : base(creationTimestamp, tags) @@ -67,7 +67,7 @@ public override bool TryGetSize(out long size) // only if we haven't already burned if (TryReserve()) { - size = _localSizeOverride >= 0 ? _localSizeOverride : _buffer.Length; + size = _localSizeOverride ?? _buffer.Length; _ = Release(); return true; } diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index 0d22553b8ff..fd368c84112 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -628,13 +628,16 @@ private void SetResult(CacheItem value, HybridCacheEntryFlags activeFlags, Ti /// The factory's write-side flags fully replace the user-supplied write-side flags, but /// are always preserved. Expiration / LocalCacheExpiration / LocalSize mutations need no /// action here because the factory wrote directly to , which is read - /// by SetL1 / SetL2Async / ResolveLocalSize. + /// by SetL1 / SetL2Async / ResolveLocalSize. LocalSize is validated via + /// since this is the only point at which factory mutations are observed. /// private static void ApplyFactoryOptions( HybridCacheEntryOptions factoryOptions, HybridCacheEntryFlags mandatoryWriteSideFlags, ref HybridCacheEntryFlags activeFlags) { + ValidateOptions(factoryOptions); + HybridCacheEntryFlags factoryFlags = factoryOptions.Flags ?? HybridCacheEntryFlags.None; activeFlags = (activeFlags & ~WriteSideFlags) | (factoryFlags & WriteSideFlags) | mandatoryWriteSideFlags; } diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs index c43b84dec5d..ddbdc188560 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs @@ -220,6 +220,8 @@ private GetOrCreateOutcome TryBeginGetOrCreate( out StampedeState? stampede, out ValueTask result) { + ValidateOptions(options); + canBeCanceled = cancellationToken.CanBeCanceled; if (canBeCanceled) { @@ -278,6 +280,15 @@ public override ValueTask RemoveAsync(string key, CancellationToken cancellation return _backendCache is null ? default : new(_backendCache.RemoveAsync(key, cancellationToken)); } + private static void ValidateOptions(HybridCacheEntryOptions? options) + { + if (options?.LocalSize is { } size && size < 0) + { + Throw.ArgumentException(nameof(options), + $"{nameof(HybridCacheEntryOptions)}.{nameof(HybridCacheEntryOptions.LocalSize)} must be non-negative."); + } + } + internal static HybridCacheEntryOptions CloneOptionsOrNew(HybridCacheEntryOptions? options) { if (options is null) @@ -304,6 +315,8 @@ internal static HybridCacheEntryOptions CloneOptionsOrNew(HybridCacheEntryOption public override ValueTask SetAsync(string key, T value, HybridCacheEntryOptions? options = null, IEnumerable? tags = null, CancellationToken cancellationToken = default) { + ValidateOptions(options); + // since we're forcing a write: disable L1+L2 read; we'll use a direct pass-thru of the value as the callback, to reuse all the code // note also that stampede token is not shared with anyone else HybridCacheEntryFlags flags = GetEffectiveFlags(options) | (HybridCacheEntryFlags.DisableLocalCacheRead | HybridCacheEntryFlags.DisableDistributedCacheRead); diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs index cbb3d7ce467..0f51306b2e6 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs @@ -20,15 +20,21 @@ namespace Microsoft.Extensions.Caching.Hybrid.Tests; // directly to the options instance and which are read by SetL1 / SetL2Async / ResolveLocalSize. public class FactoryOptionsTests(ITestOutputHelper log) : IClassFixture { - private static (DefaultHybridCache cache, CapturingCache localCache, ServiceProvider provider) BuildCacheWithL2(ITestOutputHelper log) + private static ServiceProvider GetDefaultCache(out DefaultHybridCache cache, Action? config = null) { - var localCache = new CapturingCache(new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions()))); var services = new ServiceCollection(); - services.AddSingleton(new LoggingCache(log, localCache)); + config?.Invoke(services); services.AddHybridCache(); - var provider = services.BuildServiceProvider(); - var cache = Assert.IsType(provider.GetRequiredService()); - return (cache, localCache, provider); + ServiceProvider provider = services.BuildServiceProvider(); + cache = Assert.IsType(provider.GetRequiredService()); + return provider; + } + + private static ServiceProvider BuildCacheWithL2(ITestOutputHelper log, out DefaultHybridCache cache, out CapturingCache localCache) + { + var captured = new CapturingCache(new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions()))); + localCache = captured; + return GetDefaultCache(out cache, services => services.AddSingleton(new LoggingCache(log, captured))); } private static Task WaitForBackgroundL2WriteAsync(CapturingCache cache, string key) @@ -41,27 +47,24 @@ private static Task WaitForBackgroundL2WriteAsync(CapturingCache cache, string k public async Task FactoryCanReEnableL2Write_ThatCallerDisabled() { // Caller disabled L2 writes; factory clears the flag — value must be persisted to L2. - var (cache, localCache, provider) = BuildCacheWithL2(log); - using (provider) - { - string key = nameof(FactoryCanReEnableL2Write_ThatCallerDisabled); + using var provider = BuildCacheWithL2(log, out var cache, out var localCache); + string key = nameof(FactoryCanReEnableL2Write_ThatCallerDisabled); - _ = await cache.GetOrCreateAsync( - key, - (entryOptions, _) => - { - entryOptions.Flags = HybridCacheEntryFlags.None; - return new ValueTask(Guid.NewGuid()); - }, - options: new HybridCacheEntryOptions - { - Expiration = TimeSpan.FromMinutes(1), - Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite, - }); + _ = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + entryOptions.Flags = HybridCacheEntryFlags.None; + return new ValueTask(Guid.NewGuid()); + }, + options: new HybridCacheEntryOptions + { + Expiration = TimeSpan.FromMinutes(1), + Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite, + }); - await WaitForBackgroundL2WriteAsync(localCache, key); - Assert.NotNull(localCache.Get(key)); - } + await WaitForBackgroundL2WriteAsync(localCache, key); + Assert.NotNull(localCache.Get(key)); } [Fact] @@ -69,27 +72,24 @@ public async Task FactoryCanDisableL2Write_ThatCallerEnabled() { // Symmetric tightening: caller allowed L2 writes (None), factory disables them. // Replace-semantics in ApplyFactoryOptions must make the factory's restriction stick. - var (cache, localCache, provider) = BuildCacheWithL2(log); - using (provider) - { - string key = nameof(FactoryCanDisableL2Write_ThatCallerEnabled); + using var provider = BuildCacheWithL2(log, out var cache, out var localCache); + string key = nameof(FactoryCanDisableL2Write_ThatCallerEnabled); - _ = await cache.GetOrCreateAsync( - key, - (entryOptions, _) => - { - entryOptions.Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite; - return new ValueTask(Guid.NewGuid()); - }, - options: new HybridCacheEntryOptions - { - Expiration = TimeSpan.FromMinutes(1), - Flags = HybridCacheEntryFlags.None, - }); + _ = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + entryOptions.Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite; + return new ValueTask(Guid.NewGuid()); + }, + options: new HybridCacheEntryOptions + { + Expiration = TimeSpan.FromMinutes(1), + Flags = HybridCacheEntryFlags.None, + }); - await Task.Delay(500); - Assert.Null(localCache.Get(key)); - } + await Task.Delay(500); + Assert.Null(localCache.Get(key)); } [Fact] @@ -98,10 +98,7 @@ public async Task FactoryCanEnableL1Write_ThatCallerDisabled() // L1 counterpart of FactoryCanReEnableL2Write: caller passed DisableLocalCacheWrite, // factory clears it. If the override sticks, a subsequent read returns the same value // from L1 without re-invoking the factory. - var services = new ServiceCollection(); - services.AddHybridCache(); - using var provider = services.BuildServiceProvider(); - var cache = provider.GetRequiredService(); + using var provider = GetDefaultCache(out var cache); string key = nameof(FactoryCanEnableL1Write_ThatCallerDisabled); int factoryCalls = 0; @@ -134,11 +131,7 @@ public async Task FactoryExpirationMutation_PropagatesToL2() // Factory mutates Expiration; the L2 backend must receive DistributedCacheEntryOptions // whose AbsoluteExpirationRelativeToNow matches the factory-set value. var captured = new CapturingCache(new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions()))); - var services = new ServiceCollection(); - services.AddSingleton(captured); - services.AddHybridCache(); - using var provider = services.BuildServiceProvider(); - var cache = provider.GetRequiredService(); + using var provider = GetDefaultCache(out var cache, services => services.AddSingleton(captured)); var factoryExpiration = TimeSpan.FromMinutes(7); _ = await cache.GetOrCreateAsync( @@ -164,14 +157,13 @@ public async Task FactoryLocalCacheExpirationMutation_ShortensL1Only() using var l1 = new MemoryCache(new MemoryCacheOptions { Clock = clock }); var l2 = new LoggingCache(log, new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions { Clock = clock }))); - var services = new ServiceCollection(); - services.AddSingleton(clock); - services.AddSingleton(clock); - services.AddSingleton(l1); - services.AddSingleton(l2); - services.AddHybridCache(); - using var provider = services.BuildServiceProvider(); - var cache = provider.GetRequiredService(); + using var provider = GetDefaultCache(out var cache, services => + { + services.AddSingleton(clock); + services.AddSingleton(clock); + services.AddSingleton(l1); + services.AddSingleton(l2); + }); string key = nameof(FactoryLocalCacheExpirationMutation_ShortensL1Only); int factoryCalls = 0; @@ -204,11 +196,7 @@ public async Task FactoryLocalSizeMutation_HonoredForL1SizeAccounting() // entry would be evicted from L1; the factory sets LocalSize = 1 to make the entry fit. // Verify by issuing a second call: if L1 retained the value, the factory is not // re-invoked and the same Guid is returned. - var services = new ServiceCollection(); - services.AddMemoryCache(options => options.SizeLimit = 5); - services.AddHybridCache(); - using var provider = services.BuildServiceProvider(); - var cache = provider.GetRequiredService(); + using var provider = GetDefaultCache(out var cache, services => services.AddMemoryCache(options => options.SizeLimit = 5)); string key = nameof(FactoryLocalSizeMutation_HonoredForL1SizeAccounting); int factoryCalls = 0; @@ -246,10 +234,7 @@ public async Task FactoryMutations_DoNotLeakToCallerOptionsInstance() // factory so that any mutations the factory performs do not bleed back into the caller's // shared instance. A caller that reuses the same options across many calls must see the // exact values it constructed. - var services = new ServiceCollection(); - services.AddHybridCache(); - using var provider = services.BuildServiceProvider(); - var cache = provider.GetRequiredService(); + using var provider = GetDefaultCache(out var cache); var callerOptions = new HybridCacheEntryOptions { @@ -293,28 +278,25 @@ public async Task FactoryReceivesUsableOptions_WhenCallerPassedNull() // etc. without a NullReferenceException, and the documented "mutate-in-factory" API // shape would be unusable in the common case. We observe both that the options object // is non-null and mutable, and that the mutation actually takes effect (no L2 write). - var (cache, localCache, provider) = BuildCacheWithL2(log); - using (provider) - { - string key = nameof(FactoryReceivesUsableOptions_WhenCallerPassedNull); - int factoryCalls = 0; + using var provider = BuildCacheWithL2(log, out var cache, out var localCache); + string key = nameof(FactoryReceivesUsableOptions_WhenCallerPassedNull); + int factoryCalls = 0; - _ = await cache.GetOrCreateAsync( - key, - (entryOptions, _) => - { - Interlocked.Increment(ref factoryCalls); - Assert.NotNull(entryOptions); - entryOptions.Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite; - return new ValueTask(Guid.NewGuid()); - }); + _ = await cache.GetOrCreateAsync( + key, + (entryOptions, _) => + { + Interlocked.Increment(ref factoryCalls); + Assert.NotNull(entryOptions); + entryOptions.Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite; + return new ValueTask(Guid.NewGuid()); + }); - Assert.Equal(1, factoryCalls); + Assert.Equal(1, factoryCalls); - // The factory's mutation must have taken effect — no value written to L2. - await Task.Delay(500); - Assert.Null(localCache.Get(key)); - } + // The factory's mutation must have taken effect — no value written to L2. + await Task.Delay(500); + Assert.Null(localCache.Get(key)); } [Fact] @@ -510,4 +492,20 @@ private static int StableHash(string s) return Math.Abs(h); } + + [Fact] + public async Task FactoryNegativeLocalSize_Throws() + { + using var provider = GetDefaultCache(out var cache); + + var ex = await Assert.ThrowsAsync(() => cache.GetOrCreateAsync( + nameof(FactoryNegativeLocalSize_Throws), + (entryOptions, _) => + { + entryOptions.LocalSize = -1; + return new ValueTask(0); + }).AsTask()); + + Assert.Equal("options", ex.ParamName); + } } diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FunctionalTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FunctionalTests.cs index 730013dbe4f..dd36ae1a3f3 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FunctionalTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FunctionalTests.cs @@ -78,6 +78,31 @@ public async Task RemoveMultipleKeysViaArray() Assert.Equal(96, await cache.GetOrCreateAsync(key, _ => new ValueTask(96))); } - private static string Me([CallerMemberName] string caller = "") => caller; + [Fact] + public async Task GetOrCreateAsync_RejectsNegativeLocalSize() + { + using var provider = GetDefaultCache(out var cache); + var options = new HybridCacheEntryOptions { LocalSize = -1 }; + + var ex = await Assert.ThrowsAsync( + () => cache.GetOrCreateAsync(Me(), _ => new ValueTask(42), options).AsTask()); + Assert.Equal("options", ex.ParamName); + + ex = await Assert.ThrowsAsync( + () => cache.GetOrCreateAsync(Me(), 0, (_, _, _) => new ValueTask(42), options).AsTask()); + Assert.Equal("options", ex.ParamName); + } + [Fact] + public async Task SetAsync_RejectsNegativeLocalSize() + { + using var provider = GetDefaultCache(out var cache); + var options = new HybridCacheEntryOptions { LocalSize = -1 }; + + var ex = await Assert.ThrowsAsync( + () => cache.SetAsync(Me(), 42, options).AsTask()); + Assert.Equal("options", ex.ParamName); + } + + private static string Me([CallerMemberName] string caller = "") => caller; } From f88cf1f094d09c3c3aa68c2d5c92daf800c4025d Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Fri, 12 Jun 2026 17:15:19 +0200 Subject: [PATCH 13/16] Better test failure error --- .../FactoryOptionsTests.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs index 0f51306b2e6..c8ba13b184f 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs @@ -37,10 +37,18 @@ private static ServiceProvider BuildCacheWithL2(ITestOutputHelper log, out Defau return GetDefaultCache(out cache, services => services.AddSingleton(new LoggingCache(log, captured))); } - private static Task WaitForBackgroundL2WriteAsync(CapturingCache cache, string key) + private static async Task WaitForBackgroundL2WriteAsync(CapturingCache cache, string key) { using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30)); - return cache.WaitForWriteAsync(key, cts.Token); + + try + { + await cache.WaitForWriteAsync(key, cts.Token); + } + catch (OperationCanceledException) when (cts.IsCancellationRequested) + { + Assert.Fail($"Timed out waiting for background L2 write for key '{key}'."); + } } [Fact] From dca8e902214be1513549a2ef238f4a307401aa27 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Fri, 12 Jun 2026 18:31:52 +0200 Subject: [PATCH 14/16] Honor DefaultEntryOptions.LocalSize --- .../DefaultHybridCache.StampedeStateT.cs | 5 ++- .../Internal/DefaultHybridCache.cs | 11 +++-- .../FactoryOptionsTests.cs | 45 +++++++++++++++++++ 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs index fd368c84112..63047832ffa 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs @@ -534,8 +534,9 @@ private void SetResultAndRecycleIfAppropriate(ref BufferChunk value, TimeSpan re private long? ResolveLocalSize() { // factory mutations are applied directly to _options (which is a clone). null means "use - // implementation default", per the LocalSize API contract. - return _options?.LocalSize; + // implementation default", per the LocalSize API contract — which we honor by falling + // back to HybridCacheOptions.DefaultEntryOptions.LocalSize (also nullable). + return _options?.LocalSize ?? Cache._defaultLocalSize; } private void SetImmutableResultWithoutSerialize(T value, HybridCacheEntryFlags activeFlags) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs index ddbdc188560..6102d05e899 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs @@ -26,13 +26,13 @@ internal sealed partial class DefaultHybridCache : HybridCache { internal const int DefaultExpirationMinutes = 5; - [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0032:Use auto property", Justification = "Keep usage explicit")] + [SuppressMessage("Style", "IDE0032:Use auto property", Justification = "Keep usage explicit")] private readonly IDistributedCache? _backendCache; - [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0032:Use auto property", Justification = "Keep usage explicit")] + [SuppressMessage("Style", "IDE0032:Use auto property", Justification = "Keep usage explicit")] private readonly IMemoryCache _localCache; private readonly IServiceProvider _services; // we can't resolve per-type serializers until we see each T private readonly IHybridCacheSerializerFactory[] _serializerFactories; - [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0032:Use auto property", Justification = "Keep usage explicit")] + [SuppressMessage("Style", "IDE0032:Use auto property", Justification = "Keep usage explicit")] private readonly HybridCacheOptions _options; private readonly ILogger _logger; private readonly CacheFeatures _features; // used to avoid constant type-testing @@ -42,6 +42,7 @@ internal sealed partial class DefaultHybridCache : HybridCache private readonly HybridCacheEntryFlags _defaultFlags; // note this already includes hardFlags private readonly TimeSpan _defaultExpiration; private readonly TimeSpan _defaultLocalCacheExpiration; + private readonly long? _defaultLocalSize; private readonly int _maximumKeyLength; private readonly DistributedCacheEntryOptions _defaultDistributedCacheExpiration; @@ -111,6 +112,7 @@ public DefaultHybridCache(HybridCacheOptions options, IServiceProvider services) _maximumKeyLength = _options.MaximumKeyLength; HybridCacheEntryOptions? defaultEntryOptions = _options.DefaultEntryOptions; + ValidateOptions(defaultEntryOptions); if (_backendCache is null) { @@ -120,6 +122,7 @@ public DefaultHybridCache(HybridCacheOptions options, IServiceProvider services) _defaultFlags = (defaultEntryOptions?.Flags ?? HybridCacheEntryFlags.None) | _hardFlags; _defaultExpiration = defaultEntryOptions?.Expiration ?? TimeSpan.FromMinutes(DefaultExpirationMinutes); _defaultLocalCacheExpiration = GetEffectiveLocalCacheExpiration(defaultEntryOptions) ?? _defaultExpiration; + _defaultLocalSize = defaultEntryOptions?.LocalSize; _defaultDistributedCacheExpiration = new DistributedCacheEntryOptions { AbsoluteExpirationRelativeToNow = _defaultExpiration }; #if NET9_0_OR_GREATER @@ -241,7 +244,7 @@ private GetOrCreateOutcome TryBeginGetOrCreate( if ((flags & HybridCacheEntryFlags.DisableLocalCacheRead) == 0) { - if (TryGetExisting(key, out CacheItem? typed) + if (TryGetExisting(key, out CacheItem? typed) && typed.TryGetValue(_logger, out T? value)) { if (eventSourceEnabled) diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs index c8ba13b184f..61ef31403dd 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs @@ -516,4 +516,49 @@ public async Task FactoryNegativeLocalSize_Throws() Assert.Equal("options", ex.ParamName); } + + [Fact] + public async Task DefaultEntryOptionsLocalSize_AppliedWhenCallerOmitsIt() + { + // We prove that DefaultEntryOptions are honored by setting a tight + // L1 SizeLimit + a default LocalSize=1 so a 256-byte payload (which would normally exceed + // SizeLimit and be evicted) survives in L1. A second call must hit L1 (same value back, + // factory not re-invoked) without the caller ever supplying per-call options. + string key = nameof(DefaultEntryOptionsLocalSize_AppliedWhenCallerOmitsIt); + int factoryCalls = 0; + + using var provider = GetDefaultCache(out var cache, services => + { + services.AddMemoryCache(o => o.SizeLimit = 5); + services.Configure(o => o.DefaultEntryOptions = new HybridCacheEntryOptions + { + LocalSize = 1, + Flags = HybridCacheEntryFlags.DisableDistributedCache + }); + }); + + string payload = new('x', 256); + var first = await cache.GetOrCreateAsync(key, _ => + { + Interlocked.Increment(ref factoryCalls); + return new ValueTask(payload); + }); + Assert.Equal(1, factoryCalls); + + var second = await cache.GetOrCreateAsync(key, _ => new ValueTask(Guid.NewGuid().ToString())); + Assert.Equal(first, second); + Assert.Equal(1, factoryCalls); + } + + [Fact] + public void DefaultEntryOptionsNegativeLocalSize_ThrowsAtConstruction() + { + var services = new ServiceCollection(); + services.AddHybridCache(); + services.Configure(o => o.DefaultEntryOptions = new HybridCacheEntryOptions { LocalSize = -1 }); + using var provider = services.BuildServiceProvider(); + + var ex = Assert.Throws(() => provider.GetRequiredService()); + Assert.Equal("options", ex.ParamName); + } } From 839719788a1e60c87482b66de2669a2df112de20 Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Mon, 15 Jun 2026 18:42:57 +0200 Subject: [PATCH 15/16] Less AI verbosity --- .../Internal/HybridCachePayload.cs | 4 +- .../FactoryOptionsTests.cs | 26 +++------- .../HybridCacheEventSourceTests.cs | 2 +- .../PayloadTests.cs | 50 ++++++------------- 4 files changed, 25 insertions(+), 57 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs index f36a5c8d47e..0188145ab4d 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs @@ -82,7 +82,7 @@ public static int GetMaxBytes(string key, TagSet tags, int payloadSize) + MaxVarint64Length // flags + MaxVarint64Length // payload size + MaxVarint64Length // duration - + MaxVarint64Length // optional local cache size (always budgeted; trivial over-estimate when absent) + + MaxVarint64Length // optional local cache size + MaxVarint64Length // tag count + 2 // trailing sentinel + version + GetMaxStringLength(key.Length) // key @@ -120,7 +120,7 @@ public static int Write(byte[] destination, { int payloadLength = checked((int)payload.Length); - // a negative localCacheSize is the documented "reset to default" sentinel - treat it as absent. + // a negative localCacheSize is the "reset to default" sentinel - treat it as absent. if (localCacheSize is < 0) { localCacheSize = null; diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs index 61ef31403dd..12a4aca23ad 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/FactoryOptionsTests.cs @@ -14,10 +14,7 @@ namespace Microsoft.Extensions.Caching.Hybrid.Tests; -// Covers mutations the factory makes to the HybridCacheEntryOptions it is handed — -// both Flags (replace semantics, subject to runtime-mandated floor) and non-Flags -// properties (Expiration / LocalCacheExpiration / LocalSize) that the factory writes -// directly to the options instance and which are read by SetL1 / SetL2Async / ResolveLocalSize. +// Covers mutations the factory makes to the HybridCacheEntryOptions it is handed. public class FactoryOptionsTests(ITestOutputHelper log) : IClassFixture { private static ServiceProvider GetDefaultCache(out DefaultHybridCache cache, Action? config = null) @@ -79,7 +76,6 @@ public async Task FactoryCanReEnableL2Write_ThatCallerDisabled() public async Task FactoryCanDisableL2Write_ThatCallerEnabled() { // Symmetric tightening: caller allowed L2 writes (None), factory disables them. - // Replace-semantics in ApplyFactoryOptions must make the factory's restriction stick. using var provider = BuildCacheWithL2(log, out var cache, out var localCache); string key = nameof(FactoryCanDisableL2Write_ThatCallerEnabled); @@ -202,8 +198,6 @@ public async Task FactoryLocalSizeMutation_HonoredForL1SizeAccounting() { // Tight L1 SizeLimit + a payload large enough to exceed it. Without the override the // entry would be evicted from L1; the factory sets LocalSize = 1 to make the entry fit. - // Verify by issuing a second call: if L1 retained the value, the factory is not - // re-invoked and the same Guid is returned. using var provider = GetDefaultCache(out var cache, services => services.AddMemoryCache(options => options.SizeLimit = 5)); string key = nameof(FactoryLocalSizeMutation_HonoredForL1SizeAccounting); @@ -238,7 +232,7 @@ public async Task FactoryLocalSizeMutation_HonoredForL1SizeAccounting() [Fact] public async Task FactoryMutations_DoNotLeakToCallerOptionsInstance() { - // The implementation passes a clone (or fresh instance) of the caller's options to the + // The implementation passes a clone of the caller's options to the // factory so that any mutations the factory performs do not bleed back into the caller's // shared instance. A caller that reuses the same options across many calls must see the // exact values it constructed. @@ -282,10 +276,7 @@ public async Task FactoryMutations_DoNotLeakToCallerOptionsInstance() public async Task FactoryReceivesUsableOptions_WhenCallerPassedNull() { // The options-aware overload must hand the factory a real, mutable instance even when - // the caller did not supply one — otherwise the factory cannot set entryOptions.Flags - // etc. without a NullReferenceException, and the documented "mutate-in-factory" API - // shape would be unusable in the common case. We observe both that the options object - // is non-null and mutable, and that the mutation actually takes effect (no L2 write). + // the caller did not supply one. using var provider = BuildCacheWithL2(log, out var cache, out var localCache); string key = nameof(FactoryReceivesUsableOptions_WhenCallerPassedNull); int factoryCalls = 0; @@ -310,9 +301,8 @@ public async Task FactoryReceivesUsableOptions_WhenCallerPassedNull() [Fact] public async Task FactoryLocalSize_PersistedInL2_AndReappliedOnL2Reload() { - // End-to-end exercise of commit 15d649097d: the factory-set LocalSize must be persisted - // into the L2 payload so a *different* cache instance reading from the shared L2 still - // gets the size override applied to its L1 entry. + // The factory-set LocalSize must be persisted into the L2 payload so a *different* cache + // instance reading from the shared L2 still gets the size override applied to its L1 entry. // // Setup: // Cache A: unlimited L1 SizeLimit; factory sets LocalSize=1, payload is 256 bytes. @@ -427,12 +417,12 @@ private TaskCompletionSource SignalFor(string key) => _writes.GetOrAdd(key, _ => new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)); } - // Guards against silent data loss when HybridCacheEntryOptions (owned by dotnet/runtime) - // gains a new property and the down-level branch of CloneOptionsOrNew is not updated. - // Also exercises the UnsafeAccessor path on net8.0+. [Fact] public void CloneOptionsOrNew_CopiesEveryPublicWritableProperty() { + // Guards against silent data loss when HybridCacheEntryOptions + // gains a new property and the down-level branch of CloneOptionsOrNew is not updated. + // Also exercises the UnsafeAccessor path on net8.0+. var writableProps = typeof(HybridCacheEntryOptions) .GetProperties(BindingFlags.Public | BindingFlags.Instance) .Where(p => p.CanRead && p.CanWrite) diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/HybridCacheEventSourceTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/HybridCacheEventSourceTests.cs index 8e23143475f..627695c9a6b 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/HybridCacheEventSourceTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/HybridCacheEventSourceTests.cs @@ -11,7 +11,7 @@ public class HybridCacheEventSourceTests(ITestOutputHelper log, TestEventListene { // see notes in TestEventListener for context on fixture usage - [SkippableFact] + [Fact] public void MatchesNameAndGuid() { // Assert diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs index cdb34abcf87..7c039869d4c 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs @@ -92,33 +92,6 @@ public void RoundTrip_LocalCacheSize(long? localCacheSize) ArrayPool.Shared.Return(oversized); } - [Theory] - [InlineData(-1L)] - [InlineData(-100L)] - public void RoundTrip_LocalCacheSize_NegativeSentinelIsNormalizedToAbsent(long localCacheSize) - { - // negative LocalSize is the documented "reset to default" sentinel; Write must drop it - // so the payload is emitted as v1 with no HasLocalSize flag. - using var provider = GetDefaultCache(out var cache); - - byte[] bytes = new byte[8]; - string key = "k"; - TagSet tags = TagSet.Empty; - int maxLen = HybridCachePayload.GetMaxBytes(key, tags, bytes.Length); - byte[] oversized = ArrayPool.Shared.Rent(maxLen); - - int actualLength = HybridCachePayload.Write(oversized, key, cache.CurrentTimestamp(), TimeSpan.FromMinutes(1), 0, tags, new(bytes), localCacheSize: localCacheSize); - - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, - out _, out _, out var flags, out _, out _, out long? parsedSize, out _); - - Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); - Assert.Null(parsedSize); - Assert.Equal(HybridCachePayload.PayloadFlags.None, flags & HybridCachePayload.PayloadFlags.HasLocalSize); - - ArrayPool.Shared.Return(oversized); - } - // Guards the v1 wire format. This test pins a literal v1 byte sequence and asserts the reader // still accepts it. // @@ -169,7 +142,8 @@ public void RoundTrip_SelfExpiration() Assert.Equal(1063, actualLength); clock.Add(TimeSpan.FromSeconds(58)); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); + var result = HybridCachePayload.TryParse( + new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); Assert.True(payload.SequenceEqual(bytes)); Assert.True(pendingTags.IsEmpty); @@ -205,7 +179,8 @@ public async Task RoundTrip_WildcardExpiration() clock.Add(TimeSpan.FromSeconds(2)); await cache.RemoveByTagAsync("*"); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); + var result = HybridCachePayload.TryParse( + new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.ExpiredByWildcard, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -235,7 +210,8 @@ public async Task RoundTrip_TagExpiration() clock.Add(TimeSpan.FromSeconds(2)); await cache.RemoveByTagAsync("other_tag"); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); + var result = HybridCachePayload.TryParse( + new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); Assert.True(payload.SequenceEqual(bytes)); Assert.True(pendingTags.IsEmpty); @@ -273,7 +249,8 @@ public async Task RoundTrip_TagExpiration_Pending() var tcs = new TaskCompletionSource(); cache.DebugInvalidateTag("some_tag", tcs.Task); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); + var result = HybridCachePayload.TryParse( + new(oversized, 0, actualLength), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.Success, result); Assert.True(payload.SequenceEqual(bytes)); Assert.Equal(1, pendingTags.Count); @@ -322,7 +299,8 @@ public void RoundTrip_Truncated() log.WriteLine($"bytes written: {actualLength}"); Assert.Equal(1063, actualLength); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength - 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); + var result = HybridCachePayload.TryParse( + new(oversized, 0, actualLength - 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.InvalidData, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -349,7 +327,8 @@ public void RoundTrip_Oversized() log.WriteLine($"bytes written: {actualLength}"); Assert.Equal(1063, actualLength); - var result = HybridCachePayload.TryParse(new(oversized, 0, actualLength + 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); + var result = HybridCachePayload.TryParse( + new(oversized, 0, actualLength + 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.InvalidData, result); Assert.Equal(0, payload.Count); Assert.True(pendingTags.IsEmpty); @@ -424,8 +403,7 @@ public enum FactoryMutation [InlineData(FactoryMutation.CallerDisabledL2Write_FactoryClears)] public async Task MalformedKey_DoesNotWriteToL2_EvenWhenFactoryMutatesOptions(FactoryMutation mutation) { - // When the key fails unicode validation, BackgroundFetchAsync makes sure that the (corrupted) key/tags are - // not persisted to L2. ApplyFactoryOptions must preserve that safeguard. + // When the key fails unicode validation, make sure the (corrupted) key/tags are not persisted to L2. using var collector = new LogCollector(); MemoryDistributedCache? localCache = null; using var provider = GetDefaultCache(out var cache, config => @@ -475,7 +453,7 @@ public async Task MalformedKey_DoesNotWriteToL2_EvenWhenFactoryMutatesOptions(Fa // Wait until the unicode-validation log fires (synchronous, inside BackgroundFetchAsync, // just before the L2 write decision); then give the background work a brief moment - // to flush any erroneous L2 SetAsync, mirroring the pattern in RedisTests. + // to flush any erroneous L2 SetAsync. await collector.WaitForLogsAsync([Log.IdKeyInvalidUnicode], TimeSpan.FromSeconds(5)); await Task.Delay(500); From e76ab1417f0fd5a3c8ecb5e4ead9fa37756adc5d Mon Sep 17 00:00:00 2001 From: Petr Onderka Date: Tue, 16 Jun 2026 18:12:01 +0200 Subject: [PATCH 16/16] Bounds-check TryRead7BitEncodedInt64 so truncated payloads return false Truncated/corrupt L2 payloads ending mid-varint previously read past the end of the span via buffer[index++], throwing IndexOutOfRangeException that surfaced as ParseFault instead of InvalidData. Add explicit bounds checks before each byte read. Replace the single-point truncation test with one that truncates at every length and asserts parsing never throws and never succeeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Internal/HybridCachePayload.cs | 11 ++++++++- .../PayloadTests.cs | 23 ++++++++++++++----- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs index 0188145ab4d..50d4269f161 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCachePayload.cs @@ -445,7 +445,11 @@ static bool TryRead7BitEncodedInt64(ref ReadOnlySpan buffer, out ulong res int index = 0; for (int shift = 0; shift < MaxBytesWithoutOverflow * 7; shift += 7) { - // ReadByte handles end of stream cases for us. + if (index >= buffer.Length) + { + return false; // truncated + } + byteReadJustNow = buffer[index++]; result |= (byteReadJustNow & 0x7Ful) << shift; @@ -460,6 +464,11 @@ static bool TryRead7BitEncodedInt64(ref ReadOnlySpan buffer, out ulong res // the value of this byte must fit within 1 bit (64 - 63), // and it must not have the high bit set. + if (index >= buffer.Length) + { + return false; // truncated + } + byteReadJustNow = buffer[index++]; if (byteReadJustNow > 0b_1u) { diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs index 7c039869d4c..2cce7b5efba 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/PayloadTests.cs @@ -279,8 +279,11 @@ public void Gibberish() } [Fact] - public void RoundTrip_Truncated() + public void RoundTrip_TruncatedAtEveryLength_NeverThrows() { + // Truncating the payload at *every* prefix length must surface as a clean parse result + // (never an exception that becomes ParseFault). In particular, a buffer that ends in the + // middle of a varint must not read past the end of the span. var clock = new FakeTime(); using var provider = GetDefaultCache(out var cache, config => { @@ -299,11 +302,19 @@ public void RoundTrip_Truncated() log.WriteLine($"bytes written: {actualLength}"); Assert.Equal(1063, actualLength); - var result = HybridCachePayload.TryParse( - new(oversized, 0, actualLength - 1), key, tags, cache, out var payload, out var remaining, out var flags, out var entropy, out var pendingTags, out _, out _); - Assert.Equal(HybridCachePayload.HybridCachePayloadParseResult.InvalidData, result); - Assert.Equal(0, payload.Count); - Assert.True(pendingTags.IsEmpty); + for (int truncatedLength = 0; truncatedLength < actualLength; truncatedLength++) + { + var result = HybridCachePayload.TryParse( + new(oversized, 0, truncatedLength), key, tags, cache, + out var payload, out _, out _, out _, out var pendingTags, out _, out var fault); + + Assert.Null(fault); + Assert.NotEqual(HybridCachePayload.HybridCachePayloadParseResult.Success, result); + Assert.Equal(0, payload.Count); + Assert.True(pendingTags.IsEmpty); + } + + ArrayPool.Shared.Return(oversized); } [Fact]