How can i counter a ConcurrentModificationException?

Question

if have the following problem: I have a List which i am going through using the enhanced for loop. Every time i want to remove sth, out of the list, i get a ConcurrentModificationException. I already found out why this exception is thrown, but i don`t know how i can modify my code, so that its working. This is my code:

for(Subject s : SerData.schedule)
    {
        //Checking of the class is already existing
        for(Classes c : s.classes)
        {
            if(c.day == day &c.which_class == which_class)
            {
                int index = getclassesindex(s.classes, new Classes(day, which_class));
                synchronized (s) {
                    s.classes.remove(index);

                }
            }
        }
            //More code....
    }

I also tried out this implementation.

for(Subject s : SerData.schedule)
    {
        //Checking of the class is already existing
        Iterator<Classes> x = s.classes.iterator();
        while(x.hasNext())
        {
            Classes c = x.next();
            if(c.day == day &c.which_class == which_class)
            {
                int index = getclassesindex(s.classes, new Classes(day, which_class));
                synchronized (s) {
                    s.classes.remove(index);
                }
            }
        }
        //More code....
    }

not working either...

Is there a common used, standard solution? (Hopefully sth. that is not obvious :D )

Answer 1

The main reason this issue occurs is because of the semantic meaning of your for-each loop.

When you use for-each loops, the data structure that is being traversed cannot be modified.

Essentially anything of this form will throw this exception:

for( Object o : objCollection )
{
    // ...
    if ( satisfiesSomeProperty ( o ) )
       objList.remove(o);    // This is an error!!
    // ...
}

As a side note, you can't add or replace elements in the collection either.

There are a few ways to perform this operation.

One way is to use an iterator and call the remove() method when the object is to be removed.

Iterator <Object> objItr = objCollection.iterator();

while(objItr.hasNext())
{
    Object o = objItr.next();
    // ...
    if ( satifiesSomeProperty ( o ) )
        objItr.remove();    // This is okay
    // ...
}

This option has the property that removal of the object is done in time proportional to the iterator's remove method.

The next option is to store the objects you want to remove, and then remove them after traversing the list. This may be useful in situations where removal during iteration may produce inconsistent results.

Collection <Object> objsToRemove = // ...
for( Object o : objCollection )
{
    // ...
    if ( satisfiesSomeProperty ( o ) )
       objsToRemove.add (o);
    // ...
}
objCollection.removeAll ( objsToRemove );

These two methods work for general Collection types, but for lists, you could use a standard for loop and walk the list from the end of the list to the front, removing what you please.

for (int i = objList.size() - 1; i >= 0; i--)
{
    Object o = objList.get(i);
    // ...
    if ( satisfiesSomeProperty(o) )
       objList.remove(i);
    // ...
}

Walking in the normal direction and removing could also be done, but you would have to take care of how incrementation occurs; specifically, you don't want to increment i when you remove, since the next element is shifted down to the same index.

for (int i = 0; i < objList.size(); i++)
{
    Object o = objList.get(i);
    // ...
    if ( satisfiesSomeProperty(o) )
    {
       objList.remove(i);
       i--;
    }

    //caveat: only works if you don't use `i` later here
    // ...
}

Hope this provides a good overview of the concepts and helps!

Answer 2

Using Iterator.remove() should prevent the exception from being thrown.

Answer 3

Hm if I get it right you are iterating over a collection of classes and if a given class matches some criteria you are looking for the its index and try to remove it?

Why not just do:

Iterator<Classes> x = s.classes.iterator();
while(x.hasNext()){
    Classes c = x.next();
    if(c.day == day && c.which_class == which_class) {
        x.remove();
    }
}

Add synchronization if need be (but I would prefer a concurrent collection if I were you), preferably change the "==" to equals(), add getters/setters etc. Also the convention in java is to name variables and methods using camelCase (and not separating them with "_").

Actually this is one of the cases when you have to use an iterator.

Answer 4

There is no general solution for Collection subclasses in general - most iterators will become invalid if the collection is modified, unless the modification happens through the iterator itself via Iterator.remove().

There is a potential solution when it comes to List implementations: the List interface has index-based add/get/set/remove operations. Rather than use an Iterator instance, you can iterate through the list explicitly with a counter-based loop, much like with arrays. You should take care, however, to update the loop counter appropriately when inserting or deleting elements.

Answer 5

From the javadoc on ConcurrentModificationException:

"if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will throw this exception."

So within your for (Classes c : s.classes)

you are executing s.classes.remove(index)

and the iterator is doing just what its contract says. Declare the index(es) in a scope outside the loop and remove your target after the loop is done.

Answer 6

Iterator<Classes> classesIterator = s.classes.iterator();
while (classesIterator.hasNext()) {
    Classes c = classesIterator.next();
    if (c.day == day && c.which_class == which_class) {
        classesIterator.remove();
    }
}

Answer 7

Your for-each iterator is fail-fast and this is why remove operation fails as it would change the collection while traversing it.

What implementation of List interface are you using? Noticed synchronisation on Subject, are you using this code concurrently?

If concurrency is the case, then I would recommend using CopyOnWriteArrayList. It doesn't need synchronisation and its for-each iterator doesn't throw ConcurrentModificationException.