Make the ConcurrentDictionary GetOrCreate overload atomic - #131
Merged
matt-edmondson merged 1 commit intoSep 26, 2026
Merged
Conversation
The ConcurrentDictionary overload checked with TryGetValue and then called TryAdd. When two callers missed at the same time, the loser's TryAdd returned false, which fired Debug.Assert in Debug builds. In Release it returned the loser's own defaultValue, which was never stored, so writes to it were silently lost. It now uses GetOrAdd, which returns the stored value to every caller. Fixes #130 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh
|
matt-edmondson
deleted the
claude/extensions-130-concurrent-getorcreate
branch
September 26, 2026 09:52
This was referenced Sep 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #130
Problem
DictionaryExtensions.GetOrCreate(ConcurrentDictionary<TKey,TVal>, key, defaultValue)did a non-atomic check-then-act:TryGetValue, thenTryAdd. When two callers both missed, the loser'sTryAddreturnedfalse:Debug.Assert(result)fired, which aborts the process.defaultValue, which was never stored, so anything written to it was lost.Fix
The body is now
dictionary.GetOrAdd(key, defaultValue), so every caller gets the value actually stored. TheDebug.Assertand the now-unusedSystem.Diagnosticsimport are removed. The argument validation is unchanged. I did not add the optional factory overload the issue mentions, to keep this PR to the bug.Tests
GetOrCreateConcurrentDictionaryShouldReturnStoredValueWhenAnotherCallerAddsFirstis deterministic. A comparer inserts a rival value on the second hash of the key, which is the moment between the old code's missed lookup and itsTryAdd. The test asserts the returned instance is the stored one.GetOrCreateConcurrentDictionaryShouldReturnSameInstanceToParallelCallersis the stress test from the issue. 1000Parallel.Forcallers add to aConcurrentBag, and the stored bag ends up with 1000 items.With the fix reverted:
Assert.AreSame.Debug.Assertaborts the test host (exit 134).With the fix, the full suite passes in Debug and Release (136/136).
🤖 Generated with Claude Code
https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh
Generated by Claude Code