sanderd sanderd - 10 days ago 5
C# Question

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.

Answer

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.

Comments