alastairtree / LazyCache

An easy to use thread safe in-memory caching service with a simple developer friendly API for c#
https://nuget.org/packages/LazyCache
MIT License
1.72k stars 159 forks source link

Gotcha when using CacheItemPolicy.RemovedCallback and relying on CacheEntryRemovedArguments #1

Closed tafs7 closed 9 years ago

tafs7 commented 9 years ago

I have a scenario where I create a cache key with a custom CacheItemPolicy, and I specify the RemovedCallback.

This delegate provides a parameter for the event arguments, which contains the cached item that has just been removed.

My scenario is that I'm caching a 3rd party system's login session ID value, and when that cache expires, I have to "clean up" by invoking a web service call to log out that session ID on their end.

The gotcha is that once I'm in the RemovedCallback delegate, I am no longer leveraging the LazyCache API to deal with the cache item. That means I don't get the nicety of the check for Lazy<T>, etc.

This drove me crazy for the last 30 mins, as I could see the value of the CacheEntryRemovedArguments param in the debugger, but it wasn't just my simple little int sessionID that I cached. It had been wrapped in a Lazy<int>.

It may be a good idea to provide a sample usage of the RemovedCallback that actually uses the args parameter, instead of just calling a logger, for completeness.

alastairtree commented 9 years ago

Hi

Thanks for the feedback. I have added a couple of tests and started exploring a potential solution on branch feat/policy-callbacks-should-unwrap-the-lazy. Fancy taking a look to see if the tests reproduce the issue you were experiencing?

tafs7 commented 9 years ago

Just had a chance to look at it at glance. I think you may have a bug in that fix, since you never actually assign the value from the Lazy<T>.Value property in the ternary operator

Also, the recursion on the next line will introduce a StackOverflowException:

policy.RemovedCallback = (args) =>
{
    //unwrap the cache item in a removed callback given one is specified
    if (args != null && args.CacheItem != null)
    {
        var item = args.CacheItem.Value;
        if (item is Lazy<T>)
        {
            var lazyCacheItem = (Lazy<T>) item;
            args.CacheItem.Value = lazyCacheItem.IsValueCreated ? item : null; // <-- still using the Lazy object
        }
    }
    policy.RemovedCallback(args); //<-- will cause StackOverflowException
};

I'll fork and see if I can fix that so we can get this to pass. Thanks again for getting this fix started!

alastairtree commented 9 years ago

This should be all sorted now. Version 0.6 up on nuget. Let me know any issues and thanks for the help.