Not understanding wait & notify
This test app keeps crashing and I am unsure why
publicclass ThreadTest{
public Integer counter;
/**
* @param args
*/
publicstaticvoid main(String[] args){
// TODO Auto-generated method stub
ThreadTest t =new ThreadTest();
t.run();
}
publicvoid run(){
counter =new Integer(0);
Thread workerThread;
for (int y = 0; y < 10; y++){
workerThread =new Thread(){
publicvoid run(){
try{
Thread.sleep(1000);
}catch (InterruptedException e){
// TODO Auto-generated catch block
e.printStackTrace();
}
incThreads();
try{
Thread.sleep(2000);
}catch (InterruptedException e){
// TODO Auto-generated catch block
e.printStackTrace();
}
decThreads();
}
};
workerThread.start();
}
waitOnthreads();
}
publicvoid incThreads(){
synchronized (counter){
System.out.println("inc in");
counter++;
System.out.println("inc out");
}
}
/**
* Decrement thread count
*/
publicvoid decThreads(){
synchronized (counter){
System.out.println("dec in");
counter--;
if (counter == 0){
counter.notify();
}
System.out.println("dec out");
}
}
publicvoid waitOnthreads(){
// Wait for the whole window to build
// Including the sub frames
synchronized (counter){
System.out.println("wait int");
if (counter != 0){
try{
counter.wait();
}catch (InterruptedException e1){
e1.printStackTrace();
System.exit(0);
}
}
}
System.out.println("wait out");
}
}
# 2
But when I change the synchronized to around the methods instead of code blocks it works ?
public class ThreadTest {
public Integer counter;
/**
* @param args
*/
public static void main(String[] args) {
// TODO Auto-generated method stub
ThreadTest t = new ThreadTest();
t.run();
}
public void run() {
counter = new Integer(0);
Thread workerThread;
for (int y = 0; y < 10; y++) {
workerThread = new Thread() {
public void run() {
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
incThreads();
try {
Thread.sleep(2000);
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
decThreads();
}
};
workerThread.start();
}
waitOnthreads();
}
public synchronized void incThreads() {
System.out.println("inc in");
counter++;
System.out.println("inc out");
}
/**
* Decrement thread count
*/
public synchronized void decThreads() {
System.out.println("dec in");
counter--;
if (counter == 0) {
notify();
}
System.out.println("dec out");
}
public synchronized void waitOnthreads() {
// Wait for the whole window to build
// Including the sub frames
System.out.println("wait int");
if (counter != 0) {
try {
wait();
} catch (InterruptedException e1) {
e1.printStackTrace();
System.exit(0);
}
}
System.out.println("wait out");
}
}
# 3
but this code will lead to race condition .
so what you have to do is create a object for example
Object obj = new Object();
As you synchronized on counter earlier . synchronize on this object and do wait and notify on this object.
ie your code will look like
synchronized (obj ) {
... .. . .
}
and obj.wait() or obj.notify()
hope this helps
Vijay
# 5
> Hi
> i remember reading that the synchronized object
> should not be modified , ie like in this case counter
> is modified in the synchronized block synchronized on
> counter only .
Not exactly, but the problem is related to that. It comes from that innocent looking statements like
counter++;
Note that counter is an Integer and the compiler is using autoboxing when compiling this line. The long version the compiler is generating is something like
int tmp = counter.intValue();
tmp++;
counter = new Integer(tmp);
You see, the variable counter is referring to a different object after that statement. An Integer is immutable, so for incrementing a new Integer must be created and the old one (which hold the lock) is discarede. The sync is therefore not using a single object, but another one after each inc/dec operation. This is certainly not what you want.
So using some other object for syncing or just "this" (by moving the synchronized keyword to the method declaration) seems to be a good idea.
Message was edited by:
martin@work
