System.InvalidOperationException: collection has been modified

Question

I get the following exception when listing through a queue:

System.InvalidOperationException: The collection has been modified; enumeration operation cannot be performed

here is the code snippet:

1: private bool extractWriteActions(out List<WriteChannel> channelWrites) 2: { 3: channelWrites = new List<WriteChannel>(); 4: foreach (TpotAction action in tpotActionQueue) 5: { 6: if (action is WriteChannel) 7: { 8: channelWrites.Add((WriteChannel)action); 9: lock(tpotActionQueue) 10: { 11: action.Status = RecordStatus.Batched; 12: } 13: } 14: } 15: return (channelWrites.Count > 0); 16: } 

I think I understand the problem - changing the hash table to action.Status = RecordStatus.Batched , which spins MoveNext () in the enumerator. The question is how to properly implement this "template"?

Answer 1

You can change the value in an item in the collection. The error you get means that the item has been added or removed. I .: the collection itself has been changed, not an element within the collection. This is most likely caused by another thread adding or removing items to this collection.

You must lock the queue at the beginning of your method so that other threads do not modify the collection while accessing it. Or you can lock the collection before calling this method.

 private bool extractWriteActions(out List<WriteChannel> channelWrites) { lock(tpotActionQueue) { channelWrites = new List<WriteChannel>(); foreach (TpotAction action in tpotActionQueue) { if (action is WriteChannel) { channelWrites.Add((WriteChannel)action); action.Status = RecordStatus.Batched; } } } return (channelWrites.Count > 0); } 

Answer 2

I think I had a similar exception when using the foreach in a collection where I was trying to remove items from the collection (or maybe it was a list, I don’t remember). I ended up going around it using a for loop. Perhaps try the following:

 for (int i=0; i<tpotActionQueue.Count(); i++) { TpotAction action = tpotActionQueue.Dequeue(); if (action is WriteChannel) { channelWrites.Add((WriteChannel)action); lock(tpotActionQueue) { action.Status = RecordStatus.Batched; } } } 

Answer 3

You don't have a definition for tpotActionQueue , but if it's just a normal List<TpotAction> , then this line is not your problem. Modifying a collection — adding or removing elements — does not set a property on the contained object.

You have a lock(tpotActionQueue) and thread safety tag, so I assume there is another thread adding or removing elements from tpotActionQueue during enumeration. You probably need to synchronize these calls.

Answer 4

I think all you have to do is stop using foreach and switch it to a for loop instead

 for(int i = 0; i < tpotActionQueue.Length; i++) { TpotAction action = tpotActionQueue[i]; if (action is WriteChannel) { channelWrites.Add((WriteChannel)action); lock(tpotActionQueue) { action.Status = RecordStatus.Batched; } } } 

Regards, Mike.

Answer 5

How about some good LINQy?

 private bool extractWriteActions(out List<WriteChannel> channelWrites) { channelWrites= tpotActionQueue.Where<WriteChannel>(x => x is WriteChannel).ToList() foreach(WriteChannel channel in channelWrites) { channel.Status = RecordStatus.Batched; } return ( channelWrites.Count > 0); } 

Answer 6

I think you should have some other thread modifying tpotActionQueue while you repeat it. Since you are blocking this queue inside a for loop, this is possible.