GetOrSetAsync() not refreshing for expired cache entries about cachetower

[Turnerj]

Hey @Miggleness - thanks for raising this! Looks like this is caused by 67deced where I was trying to work around a race condition. Will try and work out the best course of action to fix it without re-introducing that race condition.

[Miggleness]

I could raise a PR too if you don't already have a fix ready

[Turnerj]

Thanks for the offer! I'd normally accept it but I've already dug a bit into it now and have a fix mostly written.

CacheTower/src/CacheTower/CacheStack.cs

Lines 289 to 291 in d251195

	var previousEntry = await GetAsync<T>(cacheKey);
	if (previousEntry != default && noExistingValueAvailable && previousEntry.Expiry < DateTimeProvider.Now)
	{

The code here is to always return an existing value if it originally thought there was no value (to satisfy the race condition). What it does with previousEntry.Expiry < DateTimeProvider.Now though implies that logic will also apply for expired entries - it should not be a < but a >= to imply returning only non-expired entries.

Can't believe I missed that when I wrote it!

While the fix can be as simple as switching the sign, I'm also looking into whether I switch out the boolean noExistingValueAvailable for an enum about the status of the entry (stale, expired, miss) as it might make the surrounding code more obvious too.

[Miggleness]

cool. i was thinking along the same lines. im not sure sure about the locking part when refreshing

update: i get it now why. might be worth splitting RefreshValueAsync for each of those paths you mentioned.

[Turnerj]

Thanks again @Miggleness for raising this and providing a simple drop-in test case for the bug - little things like that make my life a lot easier! I've just released 0.11.6 which includes the fix for this bug.