xenthik xenthik - 4 months ago 13
Java Question

Why Synchronization isn't working in the following Code?

This code sometime throwing an Exception even i have used

synchronized
Method
removeFirst
within
synchronized
block of run method, I am adding and removing element on a
synchronizedList
.

public class NameDropper extends Thread {

private NameList n1;

public NameDropper(List list) {
this.n1 = new NameList(list);
}

public static void main(String[] args) {
List l = Collections.synchronizedList(new LinkedList());
NameDropper n = new NameDropper(l);
n.n1.add("Ozymandias");
Thread[] t = new NameDropper[10];
for (int i = 1; i <= 10; i++) {
t[i - 1] = new NameDropper(l);
t[i - 1].setName("T" + Integer.toString(i - 1));
t[i - 1].start();
}
}

public void run() {
synchronized (this) {
try {
Thread.sleep(50);
String name = n1.removeFirst();
System.out.println(Thread.currentThread().getName() + ": "
+ name);
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}
}
}

class NameList {
private List names = null;

public NameList(List list) {
this.names = list;
}

public synchronized void add(String name) {
names.add(name);
}

public synchronized String removeFirst() {
if (names.size() > 0)
return (String) names.remove(0);
else
return null;
}
}


The exception it is throwing:

T1: Ozymandias
T2: null
*Exception in thread "T3" java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
at java.util.LinkedList.entry(Unknown Source)
at java.util.LinkedList.remove(Unknown Source)
at java.util.Collections$SynchronizedList.remove(Unknown Source)
at NameList.removeFirst(NameDropper.java:57)*
T0: null
T8: null
*at NameDropper.run(NameDropper.java:33)*
T6: null
T4: null
T9: null
T7: null
T5: null

Answer

As pointed out by other people, you have a race condition because all of your threads are synchronized on themselves. You need a common object to synchronize on.

I would recommend that you synchronize on the list itself. It will mean that any instances that are contending for the same list are blocked on each other and any threads that are not, will not be blocked. Your add and remove methods should be:

public void add(String name) {
    synchronized (name) {
        names.add(name);
    }
}

public String removeFirst() {
    synchronized (name) {
        if (names.size() > 0)
            return (String) names.remove(0);
        else
            return null;
    }
}