Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

DictionaryExtensions.TryAdd is not thread safe #1258

Copy link
Copy link

Description

@ppittle
Issue body actions

Summary

DictionaryExtensions is susceptible to non-deterministic exceptions. Call to IDictionary.ContainsKey and IDictionary.Add need to be in a critical sections.

Failure

This has been observed in some tests, but it fails like:

Error Message:
201   System.ArgumentException : An item with the same key has already been added. Key: exampleKey
202  Stack Trace:
203     at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
204   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
205   at System.CommandLine.DictionaryExtensions.TryAdd[TKey,TValue](IDictionary`2 source, TKey key, TValue value)
206   at System.CommandLine.Collections.AliasedSet`1.Add(T item)
207   at System.CommandLine.Collections.SymbolSet.AddWithoutAliasCollisionCheck(ISymbol item)
208   at System.CommandLine.Symbol.AddParent(Symbol symbol)
209   at System.CommandLine.Command.AddSymbol(Symbol symbol)
210   at System.CommandLine.Command.Add(Symbol symbol)

Fix

Update DictionaryExtensions to include locks. Below is an example,

internal static class DictionaryExtensions
    {
        public static TValue GetOrAdd<TKey, TValue>(
            this IDictionary<TKey, TValue> source,
            TKey key,
            Func<TKey, TValue> create)
        {
            lock (source)
            {
                if (source.TryGetValue(key, out TValue value))
                {
                   return value;
                }
                else
               {
                   value = create(key);
 
                   source.Add(key, value);
 
                  return value;
            }            
        }

        public static bool TryAdd<TKey, TValue>(
            this IDictionary<TKey, TValue> source,
            TKey key,
            TValue value)
        {
            lock (source)
            {
               if (source.ContainsKey(key))
               {
                  return false;
               }
               else
               {
                  source.Add(key, value);
                  return true;
               }
            }
        }
    }

note: If this is a performance critical section, maybe tune with a double-checked lock

Reactions are currently unavailable

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

    Morty Proxy This is a proxified and sanitized view of the page, visit original site.