Java: Why ConcurrentModificationException occur with a synchronized list?

Question

With this code:

public class SynchroApp {

    public static void main(String[] args) {

        final List<String> unsyList = new ArrayList<>();
        final List<String> syList = Collections.synchronizedList(unsyList);

        TimerTask changeList = new TimerTask() {
            boolean addElem = false;

            @Override
            public void run() {
                // add / remove elements to keep size between 2 and 9
                if (syList.size() < 2)
                    addElem = true;
                else if (syList.size() > 8)
                    addElem = false;
                if (addElem)
                    syList.add(String.valueOf(System.currentTimeMillis()));
                else
                    syList.remove(0);
            }
        };

        TimerTask reverseList = new TimerTask() {
            @Override
            public void run() {
                try {
                    for (String s : syList)
                        s = new StringBuffer(s).reverse().toString();
                } catch (Exception e) {
                    System.out.println("Exception: " + e);
                }
            }
        };

        new Timer().scheduleAtFixedRate(changeList, 0L, 30L);
        new Timer().scheduleAtFixedRate(reverseList, 0L, 20L);
    }
}

Why do I still receive some ConcurrentModificationException on Iterator.next?

EDIT: the update of the list elements in reverseList wasn't working (as explained in comments). This code should work as expected:

for (int i = 0; i < syList.size(); i++)
    syList.set(i, new StringBuffer(syList.get(i)).reverse().toString());

I don't see a `next` call in that code you posted. Note, however that the loop in `reverseList`'s `run` isn't doing anything at the moment (Java Strings are immutable). — kviiri, May 02 '14 at 11:49
You cannot use an iterator and, at the same time, modify the list's content. The iterator will realize that changes were made to the list and throw this exception. — Steffen, May 02 '14 at 11:50
@kviiri Even if the class was mutable it wouldn't do anything, as it uses a temporary variable. — Alexis C., May 02 '14 at 11:52
next is implicit in (String s : syList). I think s is updated with the result of reverse(), but that's just an example anyway. — mins, May 02 '14 at 11:52
@Aru: I see now. Assumed the iterator would be smart enough, but realize this was silly. Thanks. — mins, May 02 '14 at 11:54
@ZouZou, true (but mentioning the immutability should point to the right direction!) — kviiri, May 02 '14 at 11:58
@kviiri: do you say the left hand s is temporary? I though I was using a reference to the list element... how would you code this assignment then? — mins, May 02 '14 at 12:05
@Approachingminimums, yes, `s` is a reference to the list element, but you're not altering the element of the list but changing the (local) reference to refer to a whole different String altogether. Since Strings are immutable you need to remove the old String and add a new one to actually replace something. If Strings were mutable, you could do something like `s.replace(...)` but `s = "something"` still wouldn't work. — kviiri, May 02 '14 at 12:09
Possible duplicate of [ConcurrentModificationException despite using synchronized](http://stackoverflow.com/questions/1655362/concurrentmodificationexception-despite-using-synchronized) — Raedwald, Jan 22 '16 at 16:04

score 3 · Answer 1 · answered May 02 '14 at 11:51

Because you're modifying the list while iterating on it.

Note that a synchronized list only makes each of its methods, and the methods of its iterator, synchronized. Iterating on a synchronized list is still a non-atomic operation, that involves several calls to synchronized methods. If you want to make the whole iteration atomic, you have to explicitely synchronize it, using the list itself as the lock:

synchronized (syList) {
    for (String s : syList) {
        s = new StringBuffer(s).reverse().toString();
    }
}

score 1 · Accepted Answer · answered May 02 '14 at 11:50

Even most synchronized collections do not like modifications and iterator together. From the API description of Collections.synchronizedList:

It is imperative that the user manually synchronize on the returned list when iterating over it:

List list = Collections.synchronizedList(new ArrayList()); ... synchronized (list) { Iterator i = list.iterator(); // Must be in synchronized block while (i.hasNext()) foo(i.next()); }

Also: You can use the collections from java.concurrent instead. They usually have a more refined synchronization approach.

Thanks for the reference to the API documentation. – mins May 02 '14 at 12:03 — mins, May 02 '14 at 12:03

score 0 · Answer 3 · answered Feb 28 '18 at 19:19

0

Both iterator and modifier has to use synchronizedList() in order to have proper locking/synchronization to be happened on the object.

answered Feb 28 '18 at 19:19

wonk

23
7

Java: Why ConcurrentModificationException occur with a synchronized list?

3 Answers3