Collection was modified, enumeration operation may not execute

Question

I have multithreads application and i get this error

************** Exception Text **************
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.Collections.Generic.List`1.Enumerator.MoveNext()
   ...

I probably have problem with my collection, because on one thread i read my collection and on another thread i modify collection.

public readonly ObservableCollectionThreadSafe<GMapMarker> Markers = new ObservableCollectionThreadSafe<GMapMarker>();


public void problem()
{
  foreach (GMapMarker m in Markers)
  {
    ...
  }
}

I am trying to lock collection with this code, but doesn't work.

public void problem()
    {
       lock(Markers)
       {
         foreach (GMapMarker m in Markers)
         {
           ...
         }
       }
    }

Any ideas to fix that problem?

Preside

You problem is with the code inside the foreach, please post it. – Enrol

you cant modify collection while looping with foreach – Black

What is the ObservableCollectionThreadSafe<T>? If it's a custom collection, could you include it in the question? – Stereophonic

Answer 1

This is a pretty common mistake - modifying a collection whilst iterating it using foreach, keep in mind that foreach uses readonly IEnumerator instance.

Try to loop through the collection using for() with an extra index check so if the index is out of bounds you would be able to apply additional logic to handle it. You can also use LINQ's Count() as another loop exit condition by evaluating the Count value each time if the underlying enumeration does not implement ICollection:

If Markers implements IColletion - lock on SyncRoot:

lock (Markers.SyncRoot)

Use for():

for (int index = 0; index < Markers.Count(); index++)
{
    if (Markers>= Markers.Count())
    {
       // TODO: handle this case to avoid run time exception
    }
}

You might find this post useful: How do foreach loops work in C#?

Philcox

but if the modification was by deleting an item from the collection, that will throw an IndexOutOfRange exception – Angiosperm

I mentioned extra index check to avoid this issue, will add sample, thanks for pointing to this – Philcox

i thinking to replace foreach with for but then i thought if would be better to lock collection but don't work :/ – Preside

int i = mapMarkers.Markers.IndexOf(oldMarker); if (i != -1) { mapMarkers.Markers[i] = newMarker; } – Preside

Since ObservableCollectionThreadSafe is your custom collection class, please show code for Add/Remove, BTW have you lock on this.SyncRoot inside Add/Remove methods? – Philcox

try out lock(this.SyncRoot) { this.Add(item); } inside a ObservableCollectionThreadSafe class, the same for Remove() – Philcox

GMap.NET.ObjectModel.ObservableCollectionThreadSafe<T> does not contain a definition for 'SynRoot... – Preside

Does it inherited from ObservableCollection? If so, resolve syncRoot to local field as this.syncRoot = (this as ICollection).SyncRoot since ICollection implemented explicitly – Philcox

Yep "reverting" to an ordinary for loop did the trick for me too. – Erasmo

I got exactly this error because I cleared the collection while iterating it using foreach loop as stated by @sll. The solution was to move the clearing of the collection outside the foeach loop. – Fought

Answer 2

You need to lock both on the reading and the writing side. Otherwise one of the threads will not know about the lock and will try to read/modify the collection, while the other is modifying/reading (respectively) with the lock held

Throw

Answer 3

Try to read a clone of your collection

foreach (GMapMarker m in Markers.Copy())
{
   ...
}

this will create a new copy of your collection that will not be affected by another thread but may cause a performance issue in case of huge collection.

So I think it will be better if you locked the collection while reading and writing processes.

Angiosperm

...and modify original collection. – Black

you are right, I think to use .Copy but it may cause a performance issue. – Angiosperm

Answer 4

This worked for me. Perform a ToList() operation on Markers: foreach (GMapMarker m in Markers.ToList())

Kelsi

Answer 5

Try creating a list of things to remove from the first list, then removing everything in the burner list from the first one.

// My list
List<string> myList = new List<string>();
// Burner list
List<string> burnerList = new List<string>();

// Add stuff to myList here and things to remove are added to the burner list, such as empty lines in a file

// Remove things from my list
foreach (string s in burnerList)
{
    myList.Remove(s);
}

Beleaguer

Answer 6

You can use a foreach but you have to cast the collection to a list and use the dot operator to access the behavior methods.

Example: Markers.Tolist().ForEach(i => i.DeleteObject())

Not totally sure what you're doing with your collection. My example is assuming you just wanted to delete all items from the collection, but it can be applied to any behavior you're trying to do with your collection.

Vituperate