romsky
romsky

Reputation: 884

Collections.synchronizedList and synchronized

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

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

Upvotes: 69

Views: 67371

Answers (6)

Eugene
Eugene

Reputation: 11085

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.

Upvotes: 1

Sam Goldberg
Sam Goldberg

Reputation: 6801

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

Upvotes: 100

snan
snan

Reputation: 71

Read this Oracle Doc

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

Upvotes: 7

jpegjpg
jpegjpg

Reputation: 175

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

Upvotes: 17

NPE
NPE

Reputation: 500893

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.

Upvotes: 30

assylias
assylias

Reputation: 328873

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.

Upvotes: 22

Related Questions