aspnetcore MemoryCache “magic” dependencies and danger

Posted on by

TL;DR A method whose result must be cached for 12 hours, makes a call to a method that caches for 5 minutes. This transparently causes the caller to also get a 5 minute expiry instead of the 12 hours specified!

 
var result = await memoryCache.GetOrCreateAsync(
  "mykey",
  async entry =>
  {
    entry.AbsoluteExpiration = DateTime.Now.AddHours(12);
                   
    var other = await OtherMethodThatCachesAt5Minutes();

    return "I should be cached for 12 hours! But I'm cached for 5 minutes";
});

https://github.com/aspnet/Caching/commit/f15fb804cdc2e14f9a64896817f3c6c343110820#diff-9968fc5543acfd05fa60089bd344679bR102

Track fixes/comments for the issue at:
https://github.com/aspnet/Extensions/issues/1130

Why would they deliberately do this?

There is a logic to this approach, if the child method has data that must be refreshed every 5 minutes then the parent must also ensure it is using this up-to-date information.

When would this side-effect be unwanted?

Consider the scenario:

  • ProductService calls ConfigurationService (which happens to cache its config values for 5 minutes)
  • ProductService does some long-running calculations using this configuration, including calling systems that have quota limits. The ProductService sets the AbsoluteExpiry to (now + 12 hours).

Unknown to the ProductService, its result will not be cached for 12 hours but 5 minutes, leading to more calls of the system that has quota limits.

Why is it bad?

When one cache entry is used to create another, the child copies the parent entry’s expiration tokens and time-based expiration settings. The child isn’t expired by manual removal or updating of the parent entry.

  • Testing cache expiry is notoriously difficult.
    • The MemoryCache has no methods to extract the CacheEntries to examine the actual expiry applied without resorting to reflection
    • Typically method B will be in a dependent class that is mocked during unit testing – only the combination of calls causes the issue
    • Testers are unlikely to spot the difference in timeouts between A and B or that A is being called more frequently than it should be – perhaps until the system hits production where any quotas are more likely to be hit!
  • Using side-effects is risky and doesn’t always lead to the desired behaviour.

Is there some repro for this?

Unfortunately it’s a bit wordy due to the lack of public methods on MemoryCache:

        // GET api/values
        [HttpGet]
        public async Task Get()
        {
            var result = await _memoryCache.GetOrCreateAsync("mykey",
                async entry =>
                {
                    entry.AbsoluteExpiration = DateTime.Now.AddHours(12);
                    
                    var other = await OtherMethod();

                    // And call a system that has a quota meaning I shouldn't be calling too often 
                    // ...

                    return new string[] {"value1", "value2"};
                });

            var memoryCache = typeof(Microsoft.Extensions.Caching.Memory.MemoryCache)
                .GetField("_entries", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)
                .GetValue(_memoryCache);
            var cacheDictionaryType = memoryCache.GetType();
            var cacheEntry = cacheDictionaryType.GetProperty("Item").GetValue(memoryCache, new[] { "mykey" });
            var expiration = (DateTimeOffset)cacheEntry.GetType().GetProperty("AbsoluteExpiration").GetValue(cacheEntry);
            if (expiration.Offset.TotalMinutes < 60)
            {
                throw new Exception("What? I set the absolute expiration to 12 hours");
            }

            return result;
        }

        private async Task OtherMethod()
        {
            return await _memoryCache.GetOrCreateAsync("myotherkey",
                async otherEntry =>
                {
                    otherEntry.AbsoluteExpiration = DateTime.UtcNow.Add(TimeSpan.FromMinutes(5));                    

                    return "my other data";
                });
        }

How do I ensure my AbsoluteExpiry is applied without knowing what child classes are doing? A change in their timeouts could break my code!

An excellent question! Creating a new MemoryCache does not solve the issue because no-one told these coders that “static” is a bad idea:

https://github.com/aspnet/Extensions/blob/9bc79b2f25a3724376d7af19617c33749a30ea3a/src/Caching/Memory/src/CacheEntryHelper.cs#L11

This means you can’t even create a completely separate MemoryCache as it shares a static pool of scopes. They’ve also not provided a way for you to create your own scope to work around this – all the scopes are internal.

Since I want my particular value to be long lived, I’m going for the Lazy option.

Leave a comment