The following code triggers a ConcurrentModificationException very, very quickly:
import java.util.*;
public class SynchFail {
static List<Integer> LIST = new ArrayList<Integer>();
public static void main(String[] args) {
new Thread(new Runnable() {
@Override
public void run() {
while (true) {
LIST.add(1);
}
}}).start();
new Thread(new Runnable() {
@Override
public void run() {
while (true) {
List<Integer> syncList = Collections.synchronizedList(LIST);
synchronized(syncList) {
for (Integer thisInt : syncList) {
}
}
}
}}).start();
}
}
... whereas the following behaves as it should:
import java.util.*;
public class SynchSucceed {
static List<Integer> LIST = new ArrayList<Integer>();
public static void main(String[] args) {
new Thread(new Runnable() {
@Override
public void run() {
while (true) {
synchronized(LIST) {
LIST.add(1);
}
}
}}).start();
new Thread(new Runnable() {
@Override
public void run() {
while (true) {
synchronized(LIST) {
for (Integer thisInt : LIST) {
}
}
}
}}).start();
}
}
... my understanding was that synchronized collections were to prevent ConcurrentModificationException
s in situations like this (but clearly they do not).
Given this: where should I make use of these ?
In the first code snippet, you have not followed the instructions in the documentation of synchronizedList
:
In order to guarantee serial access, it is critical that all access to the backing list is accomplished through the returned list.
In the other thread, you are adding to the list via the original LIST
, not the "returned list". LIST
is just a normal ArrayList
and calling add
on it won't acquire any locks or anything like that, so add
could still be successfully called while the iteration is in progress.
If you did:
final static List<Integer> LIST = Collections.synchronizedList(new ArrayList<>());
public static void main(String[] args) {
new Thread(new Runnable() {
@Override
public void run() {
while (true) {
LIST.add(1);
}
}}).start();
new Thread(new Runnable() {
@Override
public void run() {
while (true) {
synchronized(LIST) {
for (Integer thisInt : LIST) {
}
}
}
}}).start();
}
Then it wouldn't throw a CME. When you call add
on a synchronised list, it tries to acquire the intrinsic lock on LIST
. If iteration is in progress, the lock would have been already held by the other thread (since you did synchronized (LIST) { ... }
there), so it will wait until the iteration is over. Compare this with the second code snippet, and notice how this saves you from writing an extra synchronized (LIST) {}
block around the add
call.