Reputation: 5713
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)
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,
Upvotes: 18
Views: 15423
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
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
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
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