Ayende @ Rahien

Hi!
My name is Ayende Rahien
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

@

Posts: 5,947 | Comments: 44,540

filter by tags archive

ConcurrentDIctionary.GetOrAdd may call the valueFactory method more than once


Originally posted at 3/28/2011

When you assume, you are making an ass out of yourself, you stupid moronic idiot with no sense whatsoever. The ass in question, if anyone cares to think about it, is yours truly.

Let us take a look at the following code snippet:

var concurentDictionary = new ConcurrentDictionary<int, int>();
var w = new ManualResetEvent(false);
int timedCalled = 0;
var threads = new List<Thread>();
for (int i = 0; i < Environment.ProcessorCount; i++)
{
    threads.Add(new Thread(() =>
    {
        w.WaitOne();
        concurentDictionary.GetOrAdd(1, i1 =>
        {
            Interlocked.Increment(ref timedCalled);
            return 1;
        });
    }));
    threads.Last().Start();
}

w.Set();//release all threads to start at the same time
foreach (var thread in threads)
{
    thread.Join();
}

Console.WriteLine(timedCalled);

What would you say would be the output of this code?

Well, I assumes that it would behave in an atomic fashion, that the implementation is something like:

if(TryGetValue(key, out value))
   return value;

lock(this)
{
   if(TryGetValue(key, out value))
      return value;

   AddValue( key, valueFactory());
}

Of course, the whole point of the ConcurentDictionary is that there are no locks. Well, that is nice, except that because I assumed that the call is only made once, I called that with a function that had side effects when called twice.

That was a pure hell to figure out, because in my mind, of course that there was no error with this function.


Comments

Linkgoron

Ayende, they're not totally lock free... But the delegate executes before the lock.

BTW, is there any reason you're using Threads instead of Tasks/TPL, or is just for demonstration?

Ayende Rahien

Linkgoron,

To make sure that this actually run on separate threads.

Konstantin

There is a concept of "publish once" and "execute once" in Lazy <t, described here - msdn.microsoft.com/.../...azythreadsafetymode.aspx>

If you only need "publish once" guarantee, framework can make more performance optimizations. Unfortunately this does not seem to have been documented for ConfurrentDictionary

tobi

The reason for this is that user code should not be called while holding a lock (deadlock-prone. who knows if you function will call across threads and deadlock with itself?). Therefore the CDict will release the lock before calling your delegate. (They are internally using normal locks, but striped by hash code).

Peter Morris

I don't know what you mean by "There are no locks" - WaitOne() is a lock isn't it, or do you mean something else?

Frank Quednau

Oh, Benny Hill, awesome :)

That is a nasty one to remember.

Peter, ayende was referring to the implementation of ConcurrentDictionary

Nick Sergeev

WTF?!

I think MS should remove this method from ConcurrentDictionary.

Ara
Ara

Use ConcurrentDictionary <tkey,>

When this happens, two instances of Lazy <tvalue will be created... instead of two instances of your value type.

Ara
Ara

I meant, ConcurrentDictionary[TKey, Lazy[TValue] ]

Gian Maria Ricci

It would be nice if the documentation of GetOrAdd contains a remark telling you that the factory function could be called more than one times. I agree that we should never assume something :), but this violates the Principle of least astonishment.

Thanks Ayende for pointed this out.

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

No future posts left, oh my!

RECENT SERIES

  1. RavenDB Sharding (2):
    21 May 2015 - Adding a new shard to an existing cluster, the easy way
  2. The RavenDB Comic Strip (2):
    20 May 2015 - Part II – a team in trouble!
  3. Challenge (45):
    28 Apr 2015 - What is the meaning of this change?
  4. Interview question (2):
    30 Mar 2015 - fix the index
  5. Excerpts from the RavenDB Performance team report (20):
    20 Feb 2015 - Optimizing Compare – The circle of life (a post-mortem)
View all series

RECENT COMMENTS

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats