Skip to content
This repository has been archived by the owner. It is now read-only.

Is GetOrCreate thread-safe? #359

Closed
withinoneyear opened this issue Nov 17, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@withinoneyear
Copy link

commented Nov 17, 2017

I wonder if GetOrCreate extension method thread-safe?

From the source code it seems to perform 2 steps separately - check if key exists and then add if not. No lock is being used, so is it thread-safe?

@Tratcher

This comment has been minimized.

Copy link
Member

commented Nov 17, 2017

Do you mean to ask if it guarantees nothing else will modify the same cache key? No, it doesn't. Other threads may attempt to create an entry with the same key at the same time. Only the low level cache operations are thread safe such as Get or Set, and only in that they will not corrupt the cache. When multiple threads are operating on one key there's no guarantee which will win.

@withinoneyear

This comment has been minimized.

Copy link
Author

commented Nov 17, 2017

Thanks, I misunderstood how it works.

@amit-ezra

This comment has been minimized.

Copy link

commented Mar 25, 2018

even though this issue is closed, I think it's it should be reopened.
IMO, methods named like GetOrCreate should be atomic, i.e. thread safe. moreover the underlying ConcurrentDictionary.GetOrCreate IS thread safe, it's just is not being used in the extension method.

moreover, AFAIK the .net framework equivalent was thread safe

relevant discussion on stack overflow

@sebastienros

This comment has been minimized.

Copy link
Member

commented Mar 25, 2018

@amit-ezra
Like ConcurrentDictionary it is thread-safe. Like ConcurrentDictionary it is not atomic.

@amit-ezra

This comment has been minimized.

Copy link

commented Mar 25, 2018

@sebastienros
What I mean is - if you have multiple threads using GetOrCreate at the same time, with the same key and different value, with MemoryCache, you would get different results from each thread. which is not the desired behavior (IMO at least), and is not the same behavior you get with ConcurrentDictionary . you would expect the first thread to create, and others to get.

Maybe it's best if i give an example.
suppose you run 10 threads with the following method

private static void Worker()
    {
      var ret = memoryCache.GetOrCreate("key", (entry) => rand.Next());
      Console.WriteLine($"ret={ret}");
      Thread.Sleep(10);
    }

you would find that you get different results for ret

you run the same code (with minor adjustments) with ConcurrentDictionary.GetOrAdd you get the same ret for all threads.

I think ConcurrentDictionary is doing it the right way.

@sebastienros

This comment has been minimized.

Copy link
Member

commented Mar 25, 2018

I see, you mean the lambda can be called multiple times in both cases, but the same value it returned anyway? Unlike with MemoryCache?

@amit-ezra

This comment has been minimized.

Copy link

commented Mar 25, 2018

exactly.

@amit-ezra

This comment has been minimized.

Copy link

commented Mar 25, 2018

and this would be the same with MemoryCache.AddOrGetExisting from .net framework. and by same I mean you would get the same value from all threads (even if lambda was called multiple times)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.