ConcurrentModificationException on Iterator.next()

Question

I am getting a ConcurrentModificationException on the following code:

private static List<Task> tasks = new LinkedList<Task>();
...
public void doTasks(){
    synchronized(tasks){
        Iterator<Task> it = tasks.iterator();

        while(it.hasNext()){
            Task t = it.next(); < Exception is always thrown on this line.

            if(t.isDone()){
                it.remove();
            } else {
                t.run();
            }
        }
    }
}
...
public void addTask(Task t){
    synchronized(tasks){
        tasks.add(t);
    }
}
...
public void clearTasks(){
    synchronized(tasks){
        tasks.clear();
    }
}

The Object "tasks" is not used anywhere else in the class. I'm not sure why I'm getting the exception. Any help would be greatly appreciated.

I would use an ExecutorService. You can submit tasks to it and it is built in. — Peter Lawrey, Apr 26 '13 at 21:38
I added that information regarding where the exception is thrown into the source. — idunnololz, Apr 26 '13 at 21:42

Trinimon · Accepted Answer · 2013-04-26T21:52:33.897

3

This is your issue:

if(t.isDone()){
    ...
} else {
    t.run(); // probably changing the task, so consequently the list tasks
}

EDIT: you can't change the tasks list in the loop. Check out the ConcurrentModificationException documentation for more details.

Cheers!

edited Apr 26 '13 at 21:52

answered Apr 26 '13 at 21:39

Trinimon

13,839
9
44
60

I read elsewhere that you can if you use Iterator.remove(). http://stackoverflow.com/questions/1655362/concurrentmodificationexception-java-iterator-next – idunnololz Apr 26 '13 at 21:41
1

@idunnololz yup, you're absolutely right. `remove` is an optional operation so it can throw an `UnsupportedOperationException`, otherwise it's supported by the underlying `Collection`. – Boris the Spider Apr 26 '13 at 21:50
1

I'm inclined to say it's the `t.run()` cause the tasks `t` might be changed by `run()` ... – Trinimon Apr 26 '13 at 21:50

score 1 · Answer 2 · answered Apr 26 '13 at 21:55

1

Found the bug! I forgot the scenario where the task ran in doTask() can actually call addTask(). However I am a bit confused why this can happen as I thought the "tasks" object would be locked by the doTask() function.

answered Apr 26 '13 at 21:55

idunnololz

8,058
5
30
46

5

The `Thread` already has a lock so it works fine. `synchronized` prevents other `Thread`s from acquiring the monitor on `tasks` but the `Thread` in question already has the monitor. – Boris the Spider Apr 26 '13 at 21:56
1

Exactly what @BoristheSpider said. The locks are [reentrant](http://en.wikipedia.org/wiki/Reentrant_mutex), so if the thread tried to obtain the lock a second time, it would be granted. Ergo, Trinimon's answer was correct. – Brian Apr 26 '13 at 22:00
If they weren't reentrant, your thread would have deadlocked itself! Think about it: it would block on trying to add a task, and it wouldn't be able to unblock until it completed the doTask that's blocked by the attempt to add. It's like stopping your car at a stop sign and not starting it back up again until you're home. – yshavit Apr 26 '13 at 22:06

ConcurrentModificationException on Iterator.next()

2 Answers2

Linked