Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Modifying list from another thread while iterating (C#)

I'm looping through a List of elements with foreach, like this:

foreach (Type name in aList) {
   name.doSomething();
}

However, in an another thread I am calling something like

aList.Remove(Element);

During runtime, this causes an InvalidOperationException: Collection was modified; enumeration operation may not execute. What is the best way to handle this (I would perfer it to be rather simple even at the cost of performance)?

Thanks!

like image 264
Henrik Karlsson Avatar asked Apr 04 '12 19:04

Henrik Karlsson


2 Answers

What is the best way to handle this (I would perfer it to be rather simple even at the cost of performance)?

Fundamentally: don't try to modify a non-thread-safe collection from multiple threads without locking. The fact that you're iterating is mostly irrelevant here - it just helped you find it quicker. It would have been unsafe for two threads to both be calling Remove at the same time.

Either use a thread-safe collection such as ConcurrentBag or make sure that only one thread does anything with the collection at a time.

like image 55
Jon Skeet Avatar answered Oct 25 '22 15:10

Jon Skeet


Method #1:

The simplest and least efficient method is to create a critical section for the readers and writers.

// Writer
lock (aList)
{
  aList.Remove(item);
}

// Reader
lock (aList)
{
  foreach (T name in aList)
  {
    name.doSomething();
  }
}

Method #2:

This is similar to method #1, but instead of holding the lock for the entire duration of the foreach loop you would copy the collection first and then iterate over the copy.

// Writer
lock (aList)
{
  aList.Remove(item);
}

// Reader
List<T> copy;
lock (aList)
{
  copy = new List<T>(aList);
}
foreach (T name in copy)
{
  name.doSomething();
}

Method #3:

It all depends on your specific situation, but the way I normally deal with this is to keep the master reference to the collection immutable. That way you never have to synchronize access on the reader side. The writer side of things needs a lock. The reader side needs nothing which means the readers stay highly concurrent. The only thing you need to do is mark the aList reference as volatile.

// Variable declaration
object lockref = new object();
volatile List<T> aList = new List<T>();

// Writer
lock (lockref)
{
  var copy = new List<T>(aList);
  copy.Remove(item);
  aList = copy;
}

// Reader
List<T> local = aList;
foreach (T name in local)
{
  name.doSomething();
}
like image 26
Brian Gideon Avatar answered Oct 25 '22 17:10

Brian Gideon