Why is there a ConcurrentModificationException even when list is synchronized?

Question

I have Android multi-threading application.

There is some probability that two or more triggers might run the same part of code.

I have a list of objects.

I made it to be synchronized by Collections.synchronizedList

private List<WmGroupItemSample> mGroupItemSampleList;

mGroupItemSampleList = new ArrayList<WmGroupItemSample>();
mGroupItemSampleList = Collections.synchronizedList(mGroupItemSampleList);

However sometimes I get Exception on line:

Collections.sort(mGroupItemSampleList, new GroupItemSampleComparator());

java.util.ConcurrentModificationException
       at java.util.AbstractList$SimpleListIterator.next(AbstractList.java:62)
       at java.util.Collections.sort(Collections.java:1895)

Is this flow legal?
do I need create the copy and run sort on copy?
why Collections.synchronizedList doesn't prevent this Exception?

[EDIT]

GroupItemSampleComparator

public class GroupItemSampleComparator implements java.util.Comparator<WmGroupItemSample> {

    public GroupItemSampleComparator() {
        super();        
    }

    public int compare(WmGroupItemSample s1, WmGroupItemSample s2) {
       return ( (s2.getStartDate() - s1.getStartDate()) > 0 ) ? (-1) : (1);
    }
}

Thanks,

This exception can be reproduced *without* any [additional] threads - thus, synchronization has no bearing. — user2864740, Aug 04 '14 at 07:56
So why did you synchronize it in the first place? The answer lies in the comparator. So some code would be helpful. — Scheintod, Aug 04 '14 at 08:05
Instead of iterating over the list, give the simple for loop a try: `for (int i = 0; i < yourList.size(); i++) { Object obj = yourList.get(i); // Do something woth your object }` — Ercksen, Aug 04 '14 at 08:13
FYI: Your `Comparator` does not consider equal start dates (that might turn out to be a problem). — qqilihq, Aug 04 '14 at 08:14
Doesn't seem to be so clear now :) but try rewriting it as return s1.getStartDate() - s2.getStartDate() — Scheintod, Aug 04 '14 at 08:19
Which Java version are you using? I've looked at some Collections.java, but line 1895 isn't anywhere near sort. — laune, Aug 04 '14 at 08:20
Is you application multithreaded or not, i.e., can you exclude that any other thread accesses the List during the sort? — laune, Aug 04 '14 at 08:24
@Scheintod It would result in compilation error if `getStartDate() returns `long`. — icza, Aug 04 '14 at 08:27
@laune I have Android multi-threading application. There is some probability that two or more triggers might run the same part of code. — snaggs, Aug 04 '14 at 08:29
@PavelHoral: Because there is nothing else to focus on. If it's ConcurrentModificationException then there has to be a concurrent modification. We're trying to find it. — Scheintod, Aug 04 '14 at 09:33
Fessy already provided answer to his own question in the last comment. If he calls `Collections.sort` concurrently, it will result in `ConcurrentModificationException`. Sorting obviously uses iterators (see stack trace) to go through the collection. There is nothing special about the comparator. http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/Collections.java#177 — Pavel Horal, Aug 04 '14 at 10:26
@pavel: i remember reading the original question as if this happened without multiple threads. This is why everyone asks for the comparator. Reading the edited version it seems as if this is just "normal" concurrent modification. — Scheintod, Aug 04 '14 at 17:43
Possible duplicate of [ConcurrentModificationException despite using synchronized](http://stackoverflow.com/questions/1655362/concurrentmodificationexception-despite-using-synchronized) — Raedwald, Mar 28 '16 at 14:57

Bohemian · Accepted Answer · 2014-08-04T08:35:25.547

23

The basic problem is that a synchronized list is not synchronized in a useful way.

The problem is that while its methods are synchronized, actions like moving elements that should be atomic are not, because the separate calls needed for the move are not synchronized together. It means other threads can get in between individual method calls. Thus synchronized collections are largely deprecated now.

Despite this flaw, if you have another thread add an element while your thread is sorting, you'll get this exception because sort iterates and changing the list during iteration causes the exception.

Fortunately, the JDK has new Collection classes that have industrial strength (and useful) synchronization, courtesy of the java.util.concurrent package.

Replace your list with a CopyOnWriteArrayList, don't "synchronize" it and you'll be good to go.

edited Aug 04 '14 at 08:35

answered Aug 04 '14 at 08:29

Bohemian

412,405
93
575
722

you know that in this case I can't use `Collections.sort` – snaggs Aug 04 '14 at 08:37
it works but the problem is that "The iterator will not reflect additions, removals, or changes to the list since the iterator was created" – NullPointerException Oct 18 '19 at 10:44
"separate calls needed for the move are not synchronized together" - You can synchronize multiple method calls together by calling them within a block synchronized on the list directly. The following code sorts the list while forcing other threads to wait until the sort is finished before modifying the list: `synchronized (mGroupItemSampleList) { Collections.sort(mGroupItemSampleList, new GroupItemSampleComparator()); }` – Jack Sep 03 '21 at 09:13
@Jack but that’s exactly the *problem*. If you have a “synchronised list” you don’t expect to have to synchronise it yourself! So people don’t, and there are inevitable errors. – Bohemian Sep 03 '21 at 12:00
@Bohemian Agreed, I merely wanted to mention that it's _possible_ to perform multiple operations on a synchronized list atomically. The answer as written suggests it's not possible and that the only recourse is to use a different implementation. – Jack Sep 09 '21 at 01:46

icza · Answer 2 · 2014-08-04T09:51:57.500

Collections.synchronizedList(list) returns a synchronized list which means that the methods of the list will be synchronizd (only one of them can be running at the same time).

This however does not mean you can't call a method of the list while someone else (or maybe you) is iterating over the list using its iterator (iterators returned by iterator() are not synchronized). synchronizedList() does not protect you from getting a ConcurrentModificationException if someone is iterating over the list and it is modified in any other way than the iterator's methods.

Edit:

Also your GroupItemSampleComparator is bad, it must return 0 if the passed 2 objects are considered to be equal by their equals() method. Try this (assuming getStartDate() returns long):

public int compare(WmGroupItemSample s1, WmGroupItemSample s2) {
    long diff = s2.getStartDate() - s1.getStartDate();
    return diff > 0 ? -1 : diff < 0 ? 1 : 0;
}

score 4 · Answer 3 · answered Aug 04 '14 at 08:44

Maybe this helps - not seeing all of the code and with the chance of other accesses to the list. Quoting from Javadoc on synchronizedList(List<T> list)

Returns a synchronized (thread-safe) list backed by the specified list. In order to guarantee serial access, it is critical that all access to the backing list is accomplished through the returned list.

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());
}

So, are all iterations over this list guarded in this way?

score 3 · Answer 4 · answered Aug 04 '14 at 08:00

3

This exception does not occur only in multithreaded environments. For example, if you're iterating over a list and removing an element during an iteration, this exception can occur (depending on the way you're removing that element).

answered Aug 04 '14 at 08:00

sp00m

47,968
31
142
252

I know that but why Exception points me to `Collections.sort`? I can understand that sort process removes and appends items for sort goal. How can I get rid of that? – snaggs Aug 04 '14 at 08:02
@fessy Could you provide your `GroupItemSampleComparator`? – sp00m Aug 04 '14 at 08:04
added GroupItemSampleComparator class – snaggs Aug 04 '14 at 08:09

Why is there a ConcurrentModificationException even when list is synchronized?

4 Answers4