CODEWITHSUNDEEP
c#collectionsthread-safety
kermit_xc
kermit_xc

Reputation: 153

System.InvalidOperationException: Collection was modified

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?

Upvotes: 10

Views: 38017

Answers (6)

Kirschstein
Kirschstein

Reputation: 14868

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);
}

Upvotes: 1

Mark Brackett
Mark Brackett

Reputation: 85625

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.

Upvotes: 1

Sarah Vessels
Sarah Vessels

Reputation: 31630

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;
        }
    }
}

Upvotes: 14

Stan R.
Stan R.

Reputation: 16065

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);
   }

Upvotes: 10

CodeGoat
CodeGoat

Reputation: 1196

I think you must have some other thread modifying the tpotActionQueue while you're iterating over it. Since you're only locking that queue inside the for loop this is possible.

Upvotes: -2

Michael Ciba
Michael Ciba

Reputation: 561

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

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.

Upvotes: 1

Related Questions