Search code examples
c#.netmemorycache

AddOrGetExisting does not keep expiration into account


I am using (.NET 4.5) MemoryCache, combined with SlidingExpiration.

I notice that the method .AddOrGetExisting() does not seem to keep the expiration in mind, whilst .Get() does.

Unit tests:

    [TestMethod]
    public void NonWorking()
    {
        var memCache = new MemoryCache("somekey");
        var cachePolicy = new CacheItemPolicy() { SlidingExpiration = TimeSpan.FromSeconds(1) };

        var cacheEntry = memCache.AddOrGetExisting("key1", "foo", cachePolicy);
        Assert.AreEqual(null, cacheEntry); // OK: AddOrGetExisting returns null, because it wasn't existing yet
        Thread.Sleep(1100);

        // Expecting null, since the existing item for key1 has expired by now.
        // It is, however, still "foo".
        Assert.AreEqual(null, memCache.AddOrGetExisting("key1", "bar", cachePolicy));

        // FYI: afterwards, memCache.Get("key1") still equals "foo"
    }

    [TestMethod]
    public void Working()
    {
        var memCache = new MemoryCache("somekey");
        var cachePolicy = new CacheItemPolicy() { SlidingExpiration = TimeSpan.FromSeconds(1) };

        var cacheEntry = memCache.AddOrGetExisting("key1", "foo", cachePolicy);
        Assert.AreEqual(null, cacheEntry); // OK: AddOrGetExisting returns null, because it wasn't existing yet
        Thread.Sleep(1100);
        Assert.AreEqual(null, memCache.Get("key1"));
    }

Question:

Is this the expected behaviour for .AddOrGetExisting()?

I could fall back to .Get() and then, if null, .Add().
However, consequently I would have to implement my own locking to ensure thread-safety.


Solution

  • The problem lies in the second call of AddOrGetExisting. When calling the Method it creates a new Instance of MemoryCacheEntry SourceCode of .net Framework. In this entry it sets the Expiration to UtcNow + 1 Second, making your Thread.Sleep useless. (See this Line of the Constructor)

    I dont know why it doesnt use the policy of the existing entry to determine the timeout though. This line should use existingEntry instead of entry i guess. Maybe this is a bug?

    Here is the code in the Framework that produces the "misbehaviour"

    existingEntry = _entries[key] as MemoryCacheEntry;
    // has it expired?
    if (existingEntry != null && /* THERE => */ entry.UtcAbsExp <= DateTime.UtcNow) {
        toBeReleasedEntry = existingEntry;
        toBeReleasedEntry.State = EntryState.RemovingFromCache;
        existingEntry = null;
    }
    

    existingEntry is the "old entry" that should expire, entry is the new one with a not expired UtcAbsExp value.