Why do I get a ConcurrentModificationException?

Question

Why do I get a ConcurrentModificationException at the specified location in my code? I cannot figure out what I am doing wrong... The removeMin() method is being used to locate the min in the list pq, remove it, and return its value

import java.util.Iterator;
import java.util.LinkedList;

public class test1 {

    static LinkedList<Integer> list = new LinkedList<Integer>();

    public static void main(String[] args) {
        list.add(10);
        list.add(4);
        list.add(12);
        list.add(3);
        list.add(7);

        System.out.println(removeMin());
    }

    public static Integer removeMin() {
        LinkedList<Integer> pq = new LinkedList<Integer>();
        Iterator<Integer> itPQ = pq.iterator();

        // Put contents of list into pq
        for (int i = 0; i < list.size(); i++) {
            pq.add(list.removeFirst());
        }

        int min = Integer.MAX_VALUE;
        int pos = 0;
        int remPos = 0;

        while (itPQ.hasNext()) {
            Integer element = itPQ.next(); // I get ConcurrentModificationException here
            if (element < min) {
                min = element;
                remPos = pos;
            }
            pos++;
        }

        pq.remove(remPos);
        return remPos;
    }

}

My guess would be that since you create the `Iterator` *before* you start adding extra elements the iterator suddenly has it's data updated under it's feet, so to speak — Jeeter, Jun 08 '17 at 21:20

score 5 · Accepted Answer · answered Jun 08 '17 at 21:28

5

An Iterator should not be considered usable once the Collection from which it was obtained is modified. (This restriction is relaxed for java.util.concurrent.* collection classes.)

You are first obtaining an Iterator for pq, then modifying pq. Once you modify pq, the Iterator itPQ is no longer valid, so when you try to use it, you get a ConcurrentModificationException.

One solution is to move Iterator<Integer> itPQ = pq.iterator(); to right before the while loop. A better approach is to do away with the explicit use of Iterator altogether:

for (Integer element : pq) {

Technically, the for-each loop uses an Iterator internally, so either way, this loop would only be valid as long as you don’t try to modify pq inside the loop.

answered Jun 08 '17 at 21:28

VGR

40,506
4
48
63

upvoted for the foreach approach. Personally I think this is much cleaner – Jeeter Jun 08 '17 at 21:30
@VGR "You are first obtaining an Iterator for pq, then modifying pq". This in itself is not the issue. The issue is that the iterator's own add method was not used. to add the elements to `pq`. If you use the add method provided by ListIterator, the iterator can traverse `pq` without problems. But I like the solution. – Ishnark Jun 08 '17 at 21:38
I agree with the above statement and I have modified my code, but now when I remove the minimum value it comes up with an IndexOutOfBoundsException – kprog Jun 08 '17 at 22:48
@kprog Sounds like a different issue which should be asked as a new question. – VGR Jun 09 '17 at 09:59

Ishnark · Answer 2 · 2017-06-08T21:35:20.253

0

You face an issue because you added items to pq, using the normal .add() method after you had already created an Iterator for pq. The Iterator doesn't complain when you do the hasNext() because it sees the change in pq.

while (itPQ.hasNext()) {
    ...
    Integer element = itPQ.next(); --> you get exception here
    ...

}

However, it throws an exception when you attempt to iterate through pq. According to this post, "If the iterator detects that some modifications were made without using its method (or using another iterator on the same collection), it cannot guarantee anymore that it will not pass twice on the same element or skip one, so it throws this exception."

edited Jun 08 '17 at 21:35

answered Jun 08 '17 at 21:16

Ishnark

661
6
14

While I agree that this is definitely sketchy code, OP specifically states that the error is not thrown here – Jeeter Jun 08 '17 at 21:21
This has nothing to do with `list`. The exception is due to obtaining `itPQ`, then modifying `pq`, then trying to use `itPQ`. – VGR Jun 08 '17 at 21:23

score 0 · Answer 3 · answered Jun 08 '17 at 21:27

I ran your code, and it turns out that the offending line is here:

Iterator<Integer> itPQ = pq.iterator();

This needs to come after your population of pq, so that the iterator does not have it's data updated asynchronously.

With this modification the code runs.

Now, it does not run correctly. The reason is as @Ishnark pointed out in his answer, that every time you are removing from the list, it gets smaller, and so not all of the list is being added to pq.

Why do I get a ConcurrentModificationException?

3 Answers3

Linked

Related