snaggs
snaggs

Reputation: 5713

Why is there a ConcurrentModificationException even when list is synchronized?

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)

[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,

Upvotes: 18

Views: 15423

Answers (4)

icza
icza

Reputation: 418585

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

Upvotes: 9

laune
laune

Reputation: 31300

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?

Upvotes: 5

Bohemian
Bohemian

Reputation: 425358

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.

Upvotes: 25

sp00m
sp00m

Reputation: 48837

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).

Upvotes: 2

Related Questions