System.InvalidOperationException: Collection was modified; enumeration operation may not execute for ImmutableDictionary

Question

I have a function that is being repeatedly called via a thread. Randomly some times it raises this exception.

 System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at System.String.Join[T](String separator, IEnumerable`1 values)
   at FormatLoDestStatus(ImmutableDictionary`2 loDestStatus) 
   in C:\Formatter.cs:line 117

The function that is being called is :

  public string FormatLoDestStatus(ImmutableDictionary<string, List<int>> loDestStatus)
  {
      List<string> result = new List<string>();
      foreach (var status in loDestStatus)
      {
          string value = string.Join(",", status.Value).TrimEnd();
          var formatted = $"{status.Key}S{value}";  // This is place where it gives invalid operation
          if (Regex.IsMatch(formatted, _patternCollection.LoDestStatusPattern)
              &&
              Regex.IsMatch(value, _patternCollection.UnAvailableLevelsPattern))
          {
              result.Add(formatted);
          }
      }

      return string.Join(":", result);
  }

I converted loDestStatus from Dictionary<string, List<int>> to ImmutableDictionary<string, List<int>> but still it randomly gives exception at the line i mentioned in code where i apply string.Join(",", status.Value).TrimEnd();

What is actually going wrong and how i can solve it ? (My assumption here is if i have ImmutableDictionary, it is thread safe and the original immutable Dictionary loDestStatus shouldn't be changed).

Answer 1

You can see from your stack trace that the problem happens when you iterate the List<T> during the string.Join call:

at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
at System.String.Join[T](String separator, IEnumerable`1 values)

List<T> is not thread-safe. From the documentation:

It is safe to perform multiple read operations on a List<T>, but issues can occur if the collection is modified while it's being read. To ensure thread safety, lock the collection during a read or write operation. To enable a collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization. For collections with built-in synchronization, see the classes in the System.Collections.Concurrent namespace. For an inherently thread-safe alternative, see the ImmutableList<T> class.

So, how you want to solve this depends on how exactly you're using that list.

---

One solution is to keep using a List<T>, but make sure that you acquire a lock before reading or writing it. You could use the List<T> itself as the lock object, or have a dedicated lock:

public class ListAndLock<T>
{
    public List<T> List { get; } = new();
    public object LockObject { get; } = new();
}

Then your dictionary becomes:

ImmutableDictionary<string, ListAndLock<int>>

To add an item to the list, you need to acquire the lock first:

ListAndLock<int> listAndLock = ...;
lock (listAndLock.LockObject)
{
    listAndLock.List.Add(...);
}

And iterating the list becomes:

string value;
lock (status.Value.LockObject)
{
    value = string.Join(",", status.Value.List).TrimEnd();
}

---

Another option is to reach for a suitable thread-safe collection.

For example, if the order of items in your list doesn't actually matter, you could use a ConcurrentBag<T>:

ImmutableDictionary<string, ConcurrentBag<int>>

It is safe to read from a ConcurrentBag<T> while also adding elements to it.

If order does matter, you could make use of ConcurrentStack<T> or ConcurrentQueue<T>.

---

Another option is to go down the immutable collections route, and use an ImmutableList<T>.

This does make adding elements slightly harder, as you need to modify the ImmutableDictionary<T>:

ImmutableDictionary<string, ImmutableList<int>> dict = ...;
var newList = dict[key].Add(...);
dict = dict.SetItem(key, newList);

... and if multiple threads are doing this to the dictionary at the same time, you need to make sure that modifications made by one of the threads aren't thrown away by the other thread.