Reputation: 16908
I am trying to add String
values to an ArrayList
using two threads. What I want is that while one thread is adding the values the other thread should not interfere so I have used the Collections.synchronizedList
method. But it appears that if I don't explicitly synchronize on an object the adding is done in an unsynchronized way.
Without explicit synchronized block:
public class SynTest {
public static void main(String []args){
final List<String> list=new ArrayList<String>();
final List<String> synList=Collections.synchronizedList(list);
final Object o=new Object();
Thread tOne=new Thread(new Runnable(){
@Override
public void run() {
//synchronized(o){
for(int i=0;i<100;i++){
System.out.println(synList.add("add one"+i)+ " one");
}
//}
}
});
Thread tTwo=new Thread(new Runnable(){
@Override
public void run() {
//synchronized(o){
for(int i=0;i<100;i++){
System.out.println(synList.add("add two"+i)+" two");
}
//}
}
});
tOne.start();
tTwo.start();
}
}
The output that I got is:
true one
true two
true one
true two
true one
true two
true two
true one
true one
true one...
With the explicit synchronized block uncommented I'm stopping the interference from the other thread while adding. Once the thread has acquired the lock it is executing until it is finished.
sample output after uncommenting the synchronized block:
true one
true one
true one
true one
true one
true one
true one
true one...
So why is the Collections.synchronizedList()
not doing the synchronization?
Upvotes: 40
Views: 56287
Reputation: 1253
As @niken has described, two threads may be accessing different synchronized
methods on so called thread safe
classes. But we can not predict the order in which they will execute those synchronized
methods.
from scjp book
public class NameList{
private List names = Collections.synchronizedList(new LinkedList());
public void add(String name){
names.add(name);
}
public String removeFirst(){
if(names.size()>0)
return (String) names.remove(0);
else
return null;
}
}
Suppose this synchronized
list i.e. names
has only 1 element in it.
When Thread
t1
and t2
both lands at same time to access removeFirst()
method below are the cases
t1
acquires lock on list from synchronized method size()
.t2
waits..size()
completes (which was called by t1
), lock gets released.t2
turn to acquire that lock now for calling size()
t1
goes to remove(0)
. But can't get lock because t2
has it.size()
completes (which was called by t2
), lock gets released.t1
turn to acquire that lock now for calling remove(0)
t2
waits again, it too had to call remove(0)
, but it has no lock.removes(0)
completes (which was called by t1
), lock gets released. Element is removed and returned.t2
turn to acquire that lock now for calling remove(0)
.NoSuchElementException
for t2
.It all happens because size()
and remove()
methods together they are not atomic.
Same is the case in this question asked. Through add()
method, lock is both acquired and released, for both threads. You can not control this behavior without an explicit/external synchronized
keyword, which is commented in question.
Upvotes: 0
Reputation: 2611
There is one caveat that all of the posted answers missed. Here it is: Collections.synchronizedList will return a wrapped "thread-safe" version of a List type data structure, but it will not synchroinize operations on the list. YOU still need to synchronize operations on the backing data structure to make it really multithread-safe.
If all you do is call individual methods like add(), remove(), size(), etc. You can still get a race condition because you don't know what order those operations will be executed in, unless you synchronize them. Example
synchronize(list){
// ^ without this line the code below is not really thread-safe
while( i++ <list.size() )
if (testCondition( list.get() ) )
list.remove();
}
Upvotes: 4
Reputation: 8229
Observable behavior is absolutely correct - the synchronized
approach you demonstrate in your code sample does not do the same thing as the synchronizedList
approach.
In the first case, you synchronize the whole for
-statement, so only one thread will execute it simultaneously.
In the second case, you synchronize the collection methods themselves - that's what synchronizedList
stands for. So the add
method is synchronized - but not the for
method!
Upvotes: 3
Reputation: 1765
This is a cool little example based on the original example and accepted answer to show what purpose synchronizedList
serves.
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
public class SynTest {
public static void main(String []args) throws InterruptedException
{
final List<String> list = new ArrayList<>();
final List<String> synList = Collections.synchronizedList(new ArrayList<>());
Thread t1 = new Thread(new Runnable() {
@Override
public void run() {
list.addAll(Arrays.asList("one", "one", "one"));
synList.addAll(Arrays.asList("one", "one", "one"));
}
});
Thread t2 = new Thread(new Runnable() {
@Override
public void run() {
list.addAll(Arrays.asList("two", "two", "two"));
synList.addAll(Arrays.asList("two", "two", "two"));
}
});
t1.start();
t2.start();
Thread.sleep(1000);
System.out.println(list);
System.out.println(synList);
}
}
The original list
ends up having undefined behaviour with results such as:
[one, one, one] // wrong!
[one, one, one, null, null, null] // wrong!
[two, two, two] // wrong!
[one, one, one, two, two, two] // correct
While the synchronized synList
has a synchronised addAll
method and always produces one of the two correct results:
[one, one, one, two, two, two] // correct
[two, two, two, one, one, one] // correct
Upvotes: 9
Reputation: 3691
A synchronized list only synchronizes methods of this list.
It means a thread won't be able to modify the list while another thread is currently running a method from this list. The object is locked while processing method.
As an example, Let's say two threads run addAll
on your list, with 2 different lists (A=A1,A2,A3
and B=B1,B2,B3
) as parameter.
As the method is synchronized, you can be sure those lists won't be merged randomly like A1,B1,A2,A3,B2,B3
You don't decide when a thread handover the process to the other thread. Each method call has to fully run and return before the other one could run. So you can either get A1,A2,A3,B1,B2,B3
or B1,B2,B3,A1,A2,A3
(As we don't know which thread call will run first).
In your first piece of code, both threads runs on the same time. And both try to add
an element to the list. You don't have any way to block one thread except the synchronization on the add
method so nothing prevent thread 1 from running multiple add
operation before handing over the process to thread 2. So your output is perfectly normal.
In your second piece of code (the uncommented one), you clearly state that a thread completely lock the list from the other thread before starting the loop. Hence, you make sure one of your thread will run the full loop before the other one could access the list.
Upvotes: 58
Reputation: 982
According to previous answers you need to synchronize the synList
from access thread tOne
and tTwo
. In this case, you can use the monitor pattern to provides a security access - in order for threads.
Below I adapted your code to be share with others that have same problems. In this code I used only the synList
to control access in synchronized way. Notice that it is not necessary create other object to ensure the order access from synList.
To complement this question, see book Java Concurrency in Practice jcip chapter 4 that talks about monitor design patterns that is inspired by Hoare's work
public class SynTest {
public static void main(String []args){
final List<String> synList= Collections.synchronizedList(new ArrayList<>());
Thread tOne=new Thread(() -> {
synchronized (synList) {
for (int i = 0; i < 100; i++) {
System.out.println(synList.add("add one" + i) + " one");
try {
Thread.sleep(100);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
});
Thread tTwo=new Thread(()->{
synchronized (synList) {
for(int i=0;i<100;i++){
System.out.println(synList.add("add two"+i)+" two");
try {
Thread.sleep(200);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
});
tOne.start();
tTwo.start();
}
}
Upvotes: 2
Reputation: 44965
Collections.synchronizedList()
will synchronize all the accesses to the backed list except while iterating which still needs to be done within a synchronized block with the synchronized List instance as object's monitor.
So for example here is the code of the add
method
public boolean add(E e) {
synchronized (mutex) {return c.add(e);}
}
This guarantees serial access to the backed list, so if your 2 threads call add
at the same time, one thread will acquire the lock, add its element and release the lock then the second thread will be able to acquire the lock and add its element that is why you get alternatively one
and two
in your output.
When you uncomment the synchronized block, the code is then
synchronized(o) {
for(int i=0;i<100;i++){
...
}
}
In this case the thread that could acquire the lock on o
first will execute the entire for
loop before releasing the lock (except if an exception is thrown), allowing the other thread to execute the content of its synchronized block, which is why you get 100
consecutive times one
or two
then 100
consecutive times the other value.
Upvotes: 20