ConcurrentDictionary.GetOrAdd method executes given valueFactory concurrently

[tonyhicks20]

Description

As title states, the GetOrAdd(string key, Func valueFactory) method on the System.Collections.Concurrent.ConcurrentDictionary class calls the valueFactory multiple times.

Reproduction Steps

using System.Collections.Concurrent;

ConcurrentDictionary<string, int> dictionary = new ConcurrentDictionary<string, int>();

int count = 0;

await Task.WhenAll(
    Enumerable.Range(0, 10000)
    .Select(x => Task.Factory.StartNew(
      () => dictionary.GetOrAdd("mykey", _ => Interlocked.Increment(ref count)))
    )
);

if (count != 1)
  Console.WriteLine("This number should only be 1, but it's " + count);
else
  Console.WriteLine("Number is correct");
Console.ReadLine();

Expected behavior

I expect the value factory to only be called once.

Actual behavior

The value factory is called multiple times.

Regression?

.NET framework seems fine

Known Workarounds

No response

Configuration

The current version of .NET is 6, I tried this on 5 too and I get the same incorrect behaviour.
OS is Windows 10 - 19042.610
x64

I don't know if this is specific to this configuration but I doubt it since this works fine with .NET framework

Other information

No response

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

As title states, the GetOrAdd(string key, Func valueFactory) method on the System.Collections.Concurrent.ConcurrentDictionary class calls the valueFactory multiple times.

Reproduction Steps

`
using System.Collections.Concurrent;

ConcurrentDictionary<string, int> dictionary = new ConcurrentDictionary<string, int>();

int count = 0;

await Task.WhenAll(
Enumerable.Range(0, 10000)
.Select(x => Task.Factory.StartNew(
() => dictionary.GetOrAdd("mykey", _ => Interlocked.Increment(ref count)))
)
);

if (count != 1)
Console.WriteLine("This number should only be 1, but it's " + count);
else
Console.WriteLine("Number is correct");
Console.ReadLine();
`

Expected behavior

I expect the value factory to only be called once.

Actual behavior

The value factory is called multiple times.

Regression?

.NET framework seems fine

Known Workarounds

No response

Configuration

The current version of .NET is 6, I tried this on 5 too and I get the same incorrect behaviour.
OS is Windows 10 - 19042.610
x64

I don't know if this is specific to this configuration but I doubt it since this works fine with .NET framework

Other information

No response

Author:	tonyhicks20
Assignees:	-
Labels:	area-System.Collections, untriaged
Milestone:	-

[quixoticaxis]

The documentation explicitely describes this behavior and explains the rational behind not guarding the delegate's execution:

However, the valueFactory delegate is called outside the locks to avoid the problems that can arise from executing unknown code under a lock

Since a key/value can be inserted by another thread while valueFactory is generating a value, you cannot trust that just because valueFactory executed, its produced value will be inserted into the dictionary and returned. If you call GetOrAdd simultaneously on different threads, valueFactory may be called multiple times, but only one key/value pair will be added to the dictionary.

[danmoseley]

Thanks @quixoticaxis as you noted it's intentional and documented. Thanks @tonyhicks20 for the report though.