System.InvalidOperationException: Collection was modified
Solution 1
You are allowed to change the value in an item in a collection. The error you're getting means that an item was either added or removed i.e.: the collection itself was modified, not an item inside the collection. This is most likely caused by another thread adding or removing items to this collection.
You should lock your queue at the beginning of your method, to prevent other Threads modifying the collection while you are accessing it. Or you could lock the collection before even 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);
}
Solution 2
I think I had a similar exception when using a foreach
loop on a Collection where I tried to remove items from the Collection (or it may have been a List, I can't remember). I ended up getting around it by using a for
loop. Perhaps try something like 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;
}
}
}
Solution 3
You don't have a definition for tpotActionQueue
, but if it's just a normal List<TpotAction>
then that line is not your problem. Modifying the collection is adding or removing members - not setting a property on a contained object.
You have a lock(tpotActionQueue)
and a tag of thread-safety, so my guess is there's another thread adding or removing items from tpotActionQueue
while you're enumerating. You probably need to synchronize those accesses.
Solution 4
How about some LINQy goodness?
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);
}
Comments
-
kermit_xc about 4 years
I am getting a following exception while enumerating through a queue:
System.InvalidOperationException: Collection was modified; enumeration operation may not execute
here is the code excerpt:
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 - altering the hashtable at
action.Status = RecordStatus.Batched
, which screws up the MoveNext() on enumerator. Question is, how do I implement that "pattern" correctly? -
Stan R. almost 15 yearsthis will still cause a problem in a multi-threaded environment, it seems tpotActionQueue is a "global" variable, and another thread might modify it while this method is being called.
-
kermit_xc almost 15 yearsdang! - just verified, got a different thread pushing elements on the collection, pops are thread-safe though. Am a little worried about the 'lock' performance on the whole iteration though. It's pretty time sensitive - it might need complete rework :{. Thanks though!
-
Stan R. almost 15 yearsyes, but this method is not removing or adding items to a collection. The problem is outside of this method.
-
Stan R. almost 15 yearsalso, you're allowed to change an item in a collection, so I don't see the point of this.
-
kermit_xc almost 15 yearsThanks a lot, it does make sense ... will give it a shot.
-
atul almost 15 yearsNo Stan the problem is the action.Status assignment, read the enumeration documentation for any collection. "An enumerator remains valid as long as the collection remains unchanged. If changes are made to the collection, such as adding, modifying, or deleting elements, the enumerator is irrecoverably invalidated and its behavior is undefined". Here he's modifying an element
-
kermit_xc almost 15 yearsthat was my idea - modifying the value shouldn't screw up the collection. Ultimately I wanted to remove it from the queue, but instead chose to mark it as 'batched' so it can be taken care outside, in the main queue loop - it's the same thread though.
-
Stan R. almost 15 yearsyou've never used foreach to initialize a list? List<Test> t = new List<Test>(); t.Add(new Test()); t.Add(new Test()); foreach (Test item in t) { item.SomeName = "SomeTest"; }
-
atul almost 15 yearssetting a property does count, msdn.microsoft.com/en-us/library/4a9449ty.aspx
-
Stan R. almost 15 years@kermit_xs: "just verified, got a different thread pushing elements on the collection" - that's your issue right there. changing from "foreach" to "for" loop is not your problem and is not the answer. when using foreach you need to lock your collection before the enumeration. Enumerators are not thread-safe.
-
jes9582 almost 13 yearsthis worked perfectly... thanks a lot this saved me a lot of headache!