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.71k stars 159 forks source link

LazyCache.TryGetValue appears to return null value #164

Open thenerdynerd opened 3 years ago

thenerdynerd commented 3 years ago

There appears to be a null value returned from the out parameter when using the trygetvalue method.

The code appears to find the value in the cache if this is defined but a try get does not return a value and thus unable to be used.

Is this a bug?

gamblen commented 3 years ago

Are you evaluating the return value of the TryGetValue? If the response is false, then the out parameter would be null.

thenerdynerd commented 3 years ago

Screen Shot 2021-09-03 at 09 38 33

When then trying to evaluate the value object and or converting into the type needed it is null and fails. It does find it in the cache so it is in there and not false.

gamblen commented 3 years ago

This is exactly how I am using it and it works for me.

Would you be able to create a sample to reproduce the problem?

thenerdynerd commented 3 years ago

OK will do - for now I have had to add something like this:

Screen Shot 2021-09-03 at 09 49 35

But I will try and get you something that I have been using. Do you need to see the exception generated?

gamblen commented 3 years ago

Can you try the latest package version? That should force value to be List rather than object.

I imagine the exception is NullReference?

thenerdynerd commented 3 years ago

Yes that's right it's a null reference - just updating and will let you know. Thanks for the quick response.

thenerdynerd commented 3 years ago

Hi now it appears with the update this is not being hit or evaluated via the trygetvalue - It's the same code just the update vs non update. Is there a reason that the behaviour would change like this?

gamblen commented 3 years ago
namespace LazyCache.UnitTests
{
    using System.Collections.Generic;
    using AutoFixture;
    using FluentAssertions;
    using NUnit.Framework;

    [TestFixture]
    public class Issue164Fixture
    {
        public class MyClass
        {
            public int Id { get; set; }
            public string Name { get; set; }
        }

        [Test]
        public void Issue164()
        {
            var sut = new LazyCache.CachingService();

            var fixture = new AutoFixture.Fixture();

            var values = fixture.Create<List<MyClass>>();
            sut.Add("MyKey", values);

            if (sut.TryGetValue<List<MyClass>>("MyKey", out var cachedValues))
            {
                cachedValues.Should().BeEquivalentTo(values);
            }
            else
            {
                Assert.Fail("Not Found");
            }
        }

        [Test]
        public void Issue164_NotInCache()
        {
            var sut = new LazyCache.CachingService();

            var fixture = new AutoFixture.Fixture();

            var values = fixture.Create<List<MyClass>>();
            sut.Add("MyKey1", values);

            if (sut.TryGetValue<List<MyClass>>("MyKey2", out var cachedValues))
            {
                Assert.Fail("Found??");
            }
            else
            {
                cachedValues.Should().BeNull();
            }
        }
    }
}

How close is your implementation to this? This appears to work.

thenerdynerd commented 3 years ago

Hi thanks for this.

My implementation is very close to this I will create a quick unit test to show.

Not sure if this is relevant but I am using lazy cache in an azure API service and this is being called multiple times and sometimes at the same time. Would the cache be visible through each call?

It appears when 2 calls are close together it adds to cache but it's not committed to the cache when the second call is being invoked and getting to the trycache.

So it will fail the evaluation. Have you seen this before? Or is there anything I can do to help this? Randomly pause execution to make sure the second thread/call has committed to cache?

gamblen commented 3 years ago

Taking a guess here, but it would depend on the hosting, are you running it in an Azure Function or something else? I believe the actual caching is done by Microsoft.Extensions.Caching. I believe it is all thread safe and as it is the in memory cache it should be once added it is in. You could be having some sort of race condition of course.

An option, might be a bit dirty, is to use a lock, mutex or a semaphore to make the area so that only one thread can access the Add or TryGetValue at once.

Maybe more of a question for @alastairtree

thenerdynerd commented 3 years ago

Thanks for that - It's an Azure App service using .net Core. But I believe it is some sort of race condition. Thanks very much. If there's any tips around this would be great! Thanks again for your help.

gamblen commented 3 years ago

If you ever use the Scale out on the App Service, then the cache will be in each running VM, totally isolated. If that happens you would need to use something like Redis.

If it is all running in one process you can use lock. Inter-process would require mutex.

Sorry I have no experience running LazyCache, or any caching in a AppService.

jjjulien commented 1 year ago

I am seeing the exact same issue on an Azure App Service using .NET (Core) 6. The cache is in memory on a single instance and TryGetValue returns null whereas Get DOES return the correct value.

If I pre-load the cache with values, the TryGetValue works fine, but after any GetOrAdd with a cache miss and reload via the delegate in GetOrAdd, the TryGetValue fails for that single item and works for the rest.

bb-froggy commented 1 year ago

I have the same issue in both Azure App Services and in a local environment (NET 6) when using Get (returns the proper value) and TryGetValue (false/null) on MemoryCache, which seems also to be used in the background by LazyCache. Thus, this is an issue of MemoryCache, not LazyCache itself. I am still searching for the right place to file an issue ...

bb-froggy commented 1 year ago

Oh, in my case, I used the Generic TryGetValue, but the non-generic Get. And I used the wrong type for TryGetValue, which results in the false/null, whereas the non-generic one returned the correct object. So, my fault. Maybe it is the same issue for you, @jjjulien?

jjjulien commented 1 year ago

@bb-froggy I use generic Get and TryGetValue. The same type is being used for both. TryGetValue works w/out issue until the value is updated using the passed-in delegate, then it fails to return a value (but the Get works).

mattmpathic commented 1 year ago

I'm seeing the same failure to return a value from TryGetValue with LazyCache v2.4.0. Here is a simple repro:

var cache = new CachingService();

string key1 = "mykey1";
var value1 = cache.GetOrAdd(key1, () => "my value1");  // <-- returns "my value1"
bool result1 = cache.TryGetValue(key1, out string value1a); // <-- returns false and null
var value1b = cache.Get<string>(key1); // <-- returns "my value1"
DavidThielen commented 5 months ago

I am also finding TryGetValue() gives me some internal object while Get() works. I think the issue is (from CachingService.cs):

 public virtual T Get<T>(string key)
        {
            ValidateKey(key);

            var item = CacheProvider.Get(key);

            return GetValueFromLazy<T>(item, out _);
        }
        public virtual bool TryGetValue<T>(string key, out T value)
        {
            ValidateKey(key);

            return CacheProvider.TryGetValue(key, out value);
        }

TryGetValue is just calling the CacheProvider' rather thanGetValueFromLazy`. Is this a bug?

ian-a-anderson commented 2 months ago

Seeing the same issue, trying to write an xUnit test and TryGetValue always returns false and null even though debugging shows the values in the underlying cache. Seems like a bug.

Aaron-P commented 1 month ago

I am also finding TryGetValue() gives me some internal object while Get() works. I think the issue is (from CachingService.cs):

 public virtual T Get<T>(string key)
        {
            ValidateKey(key);

            var item = CacheProvider.Get(key);

            return GetValueFromLazy<T>(item, out _);
        }
        public virtual bool TryGetValue<T>(string key, out T value)
        {
            ValidateKey(key);

            return CacheProvider.TryGetValue(key, out value);
        }

TryGetValue is just calling the CacheProvider' rather thanGetValueFromLazy`. Is this a bug?

The type TItem being passed through to the TryGetValue of the cache provider is the type of the underlying value being cached, but that's not what LazyCache actually stores in the cache provider, at least when using GetOrAdd. It stores a wrapper object of type System.Lazy\.

image

image

This is causing the "if (result is TItem item)" pattern matching to fail and the default value to be set and return false. So the bug would seem to be that either the System.Lazy\ is being stored instead of the TItem, or the retrieval code doesn't account for the fact that it stored a System.Lazy\ instead of a TItem, depending on what the LazyCache author intended.