diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs index 47e3c5c8fd4177..6f91cb21f038cd 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs @@ -24,7 +24,7 @@ internal class CacheEntry : ICacheEntry private TimeSpan? _absoluteExpirationRelativeToNow; private TimeSpan? _slidingExpiration; private long? _size; - private IDisposable _scope; + private CacheEntry _previous; // this field is not null only before the entry is added to the cache private object _value; private int _state; // actually a [Flag] enum called "State" @@ -32,7 +32,7 @@ internal CacheEntry(object key, MemoryCache memoryCache) { Key = key ?? throw new ArgumentNullException(nameof(key)); _cache = memoryCache ?? throw new ArgumentNullException(nameof(memoryCache)); - _scope = CacheEntryHelper.EnterScope(this); + _previous = CacheEntryHelper.EnterScope(this); } /// @@ -145,11 +145,6 @@ public void Dispose() { IsDisposed = true; - // Ensure the _scope reference is cleared because it can reference other CacheEntry instances. - // This CacheEntry is going to be put into a MemoryCache, and we don't want to root unnecessary objects. - _scope.Dispose(); - _scope = null; - // Don't commit or propagate options if the CacheEntry Value was never set. // We assume an exception occurred causing the caller to not set the Value successfully, // so don't use this entry. @@ -157,11 +152,14 @@ public void Dispose() { _cache.SetEntry(this); - if (CanPropagateOptions()) + if (_previous != null && CanPropagateOptions()) { - PropagateOptions(CacheEntryHelper.Current); + PropagateOptions(_previous); } } + + CacheEntryHelper.ExitScope(this, _previous); + _previous = null; // we don't want to root unnecessary objects } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryHelper.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryHelper.cs index f7cc3a8d735e80..e06ee0fb0100ea 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryHelper.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryHelper.cs @@ -1,65 +1,32 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; +using System.Diagnostics; using System.Threading; namespace Microsoft.Extensions.Caching.Memory { - internal class CacheEntryHelper + internal static class CacheEntryHelper { - private static readonly AsyncLocal _scopes = new AsyncLocal(); - - internal static CacheEntryStack Scopes - { - get { return _scopes.Value; } - set { _scopes.Value = value; } - } + private static readonly AsyncLocal _current = new AsyncLocal(); internal static CacheEntry Current { - get - { - CacheEntryStack scopes = GetOrCreateScopes(); - return scopes.Peek(); - } + get => _current.Value; + private set => _current.Value = value; } - internal static IDisposable EnterScope(CacheEntry entry) + internal static CacheEntry EnterScope(CacheEntry current) { - CacheEntryStack scopes = GetOrCreateScopes(); - - var scopeLease = new ScopeLease(scopes); - Scopes = scopes.Push(entry); - - return scopeLease; + CacheEntry previous = Current; + Current = current; + return previous; } - private static CacheEntryStack GetOrCreateScopes() + internal static void ExitScope(CacheEntry current, CacheEntry previous) { - CacheEntryStack scopes = Scopes; - if (scopes == null) - { - scopes = CacheEntryStack.Empty; - Scopes = scopes; - } - - return scopes; - } - - private sealed class ScopeLease : IDisposable - { - private readonly CacheEntryStack _cacheEntryStack; - - public ScopeLease(CacheEntryStack cacheEntryStack) - { - _cacheEntryStack = cacheEntryStack; - } - - public void Dispose() - { - Scopes = _cacheEntryStack; - } + Debug.Assert(Current == current, "Entries disposed in invalid order"); + Current = previous; } } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryStack.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryStack.cs deleted file mode 100644 index 9b6a9b71abf41c..00000000000000 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryStack.cs +++ /dev/null @@ -1,40 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; - -namespace Microsoft.Extensions.Caching.Memory -{ - internal class CacheEntryStack - { - private readonly CacheEntryStack _previous; - private readonly CacheEntry _entry; - - private CacheEntryStack() - { - } - - private CacheEntryStack(CacheEntryStack previous, CacheEntry entry) - { - if (previous == null) - { - throw new ArgumentNullException(nameof(previous)); - } - - _previous = previous; - _entry = entry; - } - - public static CacheEntryStack Empty { get; } = new CacheEntryStack(); - - public CacheEntryStack Push(CacheEntry c) - { - return new CacheEntryStack(this, c); - } - - public CacheEntry Peek() - { - return _entry; - } - } -} diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs index e9ab2c5c26f502..e9e7fb4ebfc552 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs @@ -214,17 +214,23 @@ public void DisposingCacheEntryReleasesScope() object GetScope(ICacheEntry entry) { return entry.GetType() - .GetField("_scope", BindingFlags.NonPublic | BindingFlags.Instance) + .GetField("_previous", BindingFlags.NonPublic | BindingFlags.Instance) .GetValue(entry); } var cache = CreateCache(); - ICacheEntry entry = cache.CreateEntry("myKey"); - Assert.NotNull(GetScope(entry)); + ICacheEntry first = cache.CreateEntry("myKey1"); + Assert.Null(GetScope(first)); // it's the first entry, so it has no previous cache entry set - entry.Dispose(); - Assert.Null(GetScope(entry)); + ICacheEntry second = cache.CreateEntry("myKey2"); + Assert.NotNull(GetScope(second)); // it's not first, so it has previous set + Assert.Same(first, GetScope(second)); // second.previous is set to first + + second.Dispose(); + Assert.Null(GetScope(second)); + first.Dispose(); + Assert.Null(GetScope(first)); } [Fact]