Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LazyCache.TryGetValue appears to return null value #164

Open
thenerdynerd opened this issue Sep 2, 2021 · 20 comments
Open

LazyCache.TryGetValue appears to return null value #164

thenerdynerd opened this issue Sep 2, 2021 · 20 comments

Comments

@thenerdynerd
Copy link

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
Copy link
Contributor

gamblen commented Sep 3, 2021

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

@thenerdynerd
Copy link
Author

thenerdynerd commented Sep 3, 2021

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
Copy link
Contributor

gamblen commented Sep 3, 2021

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
Copy link
Author

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
Copy link
Contributor

gamblen commented Sep 3, 2021

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

I imagine the exception is NullReference?

@thenerdynerd
Copy link
Author

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

@thenerdynerd
Copy link
Author

thenerdynerd commented Sep 3, 2021

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
Copy link
Contributor

gamblen commented Sep 3, 2021

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
Copy link
Author

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
Copy link
Contributor

gamblen commented Sep 3, 2021

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
Copy link
Author

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
Copy link
Contributor

gamblen commented Sep 3, 2021

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
Copy link

jjjulien commented Sep 28, 2022

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
Copy link

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
Copy link

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
Copy link

jjjulien commented Oct 13, 2022

@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
Copy link

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
Copy link

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 than GetValueFromLazy`. Is this a bug?

@ian-a-anderson
Copy link

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
Copy link

Aaron-P commented Aug 4, 2024

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 than GetValueFromLazy`. 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<TItem>.

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<TItem> is being stored instead of the TItem, or the retrieval code doesn't account for the fact that it stored a System.Lazy<TItem> instead of a TItem, depending on what the LazyCache author intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants