Collections.synchronizedList and synchronized

Question

List<String> list = Collections.synchronizedList(new ArrayList<String>());
synchronized (list) {
    list.add("message");
}

Is the block "synchronized (list){} " really need here ?

Answer 1

It depends on the exact contents of the synchronized block:

1. If the block performs a single, atomic operation on the list (as in your example), the synchronized is superfluous.

2. If the block performs multiple operations on the list -- and needs to maintain the lock for the duration of the compound operation -- then the synchronized is not superfluous. One common example of this is iterating over the list.

Answer 2

You don't need to synchronize as you put in your example. HOWEVER, very important, you need to synchronize around the list when you iterate it (as noted in the Javadoc):

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

Answer 3

Also Important to note that any methods that use Iterators for example Collections.sort() will also need to be encapsulated inside a synchronized block.

Answer 4

Like what has been mentioned by others, the synchronized collections are thread-safe, but the compound actions to these collections are not guaranteed to be thread-safe by default.

According to JCIP, the common compound actions can be

• iteration
• navigation
• put-if-absent
• check-then-act

The OP's synchronized code block isn't a compound action, so no difference whether add it or not.

Let's take the example from JCIP and modify it a little to clarify why it's necessary to guard the compound actions with lock.

There are two methods that operate on same collection list that wrapped by Collections.synchronizedList

public Object getLast(List<String> list){
    int lastIndex = list.size() - 1;
    return list.get(lastIndex);
}

public void deleteLast(List<String> list){
    int lastIndex = list.size() - 1;
    list.remove(lastIndex);
}

If methods getLast and deleteLast are called at the same time by two different threads, below interleaves may happen and getLast will throw ArrayIndexOutOfBoundsException. Assume current lastIndex is 10.

Thread A (deleteLast) --> remove
Thread B (getLast) --------------------> get

The Thread A remove the element before the get operation in Thread B. Thus, the Thread B still use 10 as the lastIndex to call list.get method, it will lead to concurrent problem.

Answer 5

The underlying code for Collections.synchronizedList add method is:

public void add(int index, E element) {
    synchronized (mutex) {list.add(index, element);}
}

So in your example it is not needed to add synchronisation.

Answer 6

Read this Oracle Doc

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