13

Perhaps someone can point me in the correct direction, because I'm completely stumped on this.

I have a function that simply prints out a LinkedList of classes:

    LinkedList<Component> components = new LinkedList<Component>();
    ...
    private void PrintComponentList()
    {
        Console.WriteLine("---Component List: " + components.Count + " entries---");
        foreach (Component c in components)
        {
            Console.WriteLine(c);
        }
        Console.WriteLine("------");
    }

The Component object actually has a custom ToString() call as such:

    int Id;
    ...
    public override String ToString()
    {
        return GetType() + ": " + Id;
    }

This function typically works fine - however I've run into the issue that when it builds to about 30 or so entries in the list, the PrintcomplentList foreach statement comes back with an InvalidOperationException: Collection was modified after the enumerator was instantiated.

Now as you can see I'm not modifying the code within the for loop, and I haven't explicitly created any threads, although this is within an XNA environment (if it matters). It should be noted that the printout is frequent enough that the Console output is slowing down the program as a whole.

I'm completely stumped, has anyone else out there run into this?

gturri
  • 13,807
  • 9
  • 40
  • 57
cyberconte
  • 2,371
  • 4
  • 21
  • 27
  • That does sound extremely odd. Could you post a short but *complete* program so we can try to reproduce it? – Jon Skeet May 10 '09 at 06:59
  • I cna't get a small program to replicate the behavior, so i'm going to see about getting a thread-safe implementation of LinkedList in there to see if it catches anything – cyberconte May 12 '09 at 05:25

4 Answers4

14

I suspect the place to start looking will be at any places where you manipulate the list - i.e. insert/remove/re-assign items. My suspicion is that there will be a callback/even-handler somewhere that is getting fired asynchronously (perhaps as part of the XNA paint etc loops), and which is editing the list - essentially causing this problem as a race condition.

To check if this is the case, put some debug/trace output around the places that manipulate the list, and see if it ever (and in particular, just before the exception) runs the manipulation code at the same time as your console output:

private void SomeCallback()
{
   Console.WriteLine("---Adding foo"); // temp investigation code; remove
   components.AddLast(foo);
   Console.WriteLine("---Added foo"); // temp investigation code; remove
}

Unfortunately, such things are often a pain to debug, as changing the code to investigate it often changes the problem (a Heisenbug).

One answer would be to synchronize access; i.e. in all the places that edit the list, use a lock around the complete operation:

LinkedList<Component> components = new LinkedList<Component>();
readonly object syncLock = new object();
...
private void PrintComponentList()
{
    lock(syncLock)
    { // take lock before first use (.Count), covering the foreach
        Console.WriteLine("---Component List: " + components.Count
              + " entries---");
        foreach (Component c in components)
        {
           Console.WriteLine(c);
        }
        Console.WriteLine("------");
    } // release lock
}

and in your callback (or whatever)

private void SomeCallback()
{
   lock(syncLock)
   {
       components.AddLast(foo);
   }
}

In particular, a "complete operation" might include:

  • check the count and foreach/for
  • check for existance and insert/remove
  • etc

(i.e. not the individual/discrete operations - but units of work)

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • note also that logging the thread-id when you write your trace lines may help you identify if there are competing threads, and thus the possibility of a race. – Marc Gravell May 10 '09 at 09:20
4

Instead of foreach, I use while( collection.count >0) then use collection[i].

akjoshi
  • 15,374
  • 13
  • 103
  • 121
arash
  • 49
  • 1
  • 1
3

I don't know if this is relevant to the OP but I had the same error and found this thread during a google search. I was able to solve it by adding a break after removing an element in the loop.

foreach( Weapon activeWeapon in activeWeapons ){

            if (activeWeapon.position.Z < activeWeapon.range)
            {
                activeWeapons.Remove(activeWeapon);
                break; // Fixes error
            }
            else
            {
                activeWeapon.position += activeWeapon.velocity;
            }
        }
    }

If you leave out the break, you will get the error "InvalidOperationException: Collection was modified after the enumerator was instantiated."

Steffan
  • 307
  • 5
  • 16
  • 1
    This introduces another bug, though -- any of your weapons in the list after the first out-of-range weapon won't have their positions modified! I've dealt with this in the past by creating a second List of stuff to remove from the first list, populating it in one foreach, then doing a foreach over the second list and removing each item in it from the first list. – twon33 Aug 19 '10 at 20:42
  • This fixed my issue. – mandy1339 Jan 02 '21 at 21:31
0

Using Break could be a way but it may impact your series of operation. What I do in that case in simply convert the foreach to traditional for loop

for(i=0; i < List.count; i++)
{
    List.Remove();
    i--;
}

This works without any issues.

Yitzchak
  • 3,303
  • 3
  • 30
  • 50
Saikat Chakraborty
  • 271
  • 1
  • 3
  • 11
  • That could work, but in this case, the iteration is over objects of a collection. foreach(var obj in collection.keys)... – Futureproof Nov 22 '17 at 21:07