4

I am still a java newbie and trying to play around learning threads. My question is that it does not loop 5 times. It runs one time and exits. I am using a.class to lock on the class object, such that both the threads are locking on the same object monitor.

class a implements Runnable {
  Thread thr;
  int count;
  String time;

  a(String s) {
    thr = new Thread(this, s);
    thr.start();
  }

  public void run() {
    count++;

    if (Thread.currentThread().getName().compareTo("one") == 0) {
      synchronized (a.class) {

        try {
          for (int i = 0; i < 5; i++) {
            System.out.println("Now running thread " + Thread.currentThread().getName() + " with count " + count);

            time = "Tick";
            System.out.println(time);
            notify();

            while (time == "Tock") {
              wait();
            }
          }
        } catch (Exception e) {
        }

      }
    } else if (Thread.currentThread().getName().compareTo("two") == 0) {
      synchronized (a.class) {
        try {
          for (int j = 0; j < 5; j++) {
            System.out.println("Now running thread " + Thread.currentThread().getName() + " with count " + count);

            time = "Tock";
            System.out.println(time);
              notify();

            while (time == "Tick") {
              wait();
            }
          }
        } catch (Exception e) {
        }
      }
    }
  }
}

public class b {
  public static void main(String args[]) {

    a obj1 = new a("one");
    a obj2 = new a("two");
  }
}
Improve this question
4
  • 1
    Your code is really hard to read due to the lack of indentation. Please edit your post accordingly. You should also avoid catch (Exception e){} which may well be hiding what's going wrong. Commented Sep 11, 2012 at 23:24
  • @ Jon...you are right it is actually throwing an IllegalMonitorStateException Commented Sep 11, 2012 at 23:29
  • I don't get what you are trying to do with the wait and notify. You can only call wait on instances you got a monitor on. Since you don't have one on Thread.currentThread() this has to fail. Commented Sep 11, 2012 at 23:33
  • @Alex...agreed. I can very well just call wait() and notify(). But am I locking on the object monitor by synchronizing on 'a.class'? Commented Sep 11, 2012 at 23:47

3 Answers 3

Reset to default
3
+50

Here you go, with the original code:

class a implements Runnable {
    Thread thr;
    int count;
    static String time = "Tock";

    a(String s) {
        thr = new Thread(this, s);
        thr.start();
    }

    public void run() {
        count++;

        if (Thread.currentThread().getName().compareTo("one") == 0) {
            synchronized (a.class) {

                try {
                    for (int i = 0; i < 5; i++) {
                        while (time.equals("Tock")) {
                            a.class.wait();
                        }

                        System.out.println("Now running thread "
                                + Thread.currentThread().getName()
                                + " with count " + count);

                        time = "Tock";
                        System.out.println(time);
                        a.class.notify();                       
                    }
                } catch (Exception e) {
                    e.printStackTrace();
                }

            }
        } else if (Thread.currentThread().getName().compareTo("two") == 0) {
            synchronized (a.class) {
                try {
                    for (int j = 0; j < 5; j++) {
                        while (time.equals("Tick")) {
                            a.class.wait();
                        }

                        System.out.println("Now running thread "
                                + Thread.currentThread().getName()
                                + " with count " + count);

                        time = "Tick";
                        System.out.println(time);
                        a.class.notify();                       
                    }
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        }
    }
}

public class Test {
    public static void main(String args[]) {

        a obj1 = new a("one");
        a obj2 = new a("two");
    }
}

The problem was that you were calling wait and notify on the implicit this object, when the lock was being held on the a.class object, hence you must call wait/notify on a.class. That was it.

I also did a small restructuring, since I assume you wanted them to print Tick and Tock in an alternating sequence, right?

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

'wait( )' tells the calling thread to give up the monitor and go to sleep until some other thread enters the same monitor and calls notify( ). 'notify( )' wakes up the first thread that called wait( ) on the same object but won't give up the monitor till it reaches 'wait()'. So this will ensure always there will be only one thread will be active/working on synchronized code(shared resources).
2

The answer to why you only loop once is that you call notify() on an object that is not locked and thus an IllegalMonitorStateException is thrown and caught by the empty catch statement.

This is one way to do it. Not saying that it is the best. I tried to keep it close to your code:

public class TickTock {
    static final int N = 4;

    Object lock = new Object();
    int token;

    class Worker extends Thread {
        int id;

        Worker(int id) {
            this.id = id;
        }

        @Override
        public void run() {
            try {
                synchronized (lock) {
                    for (int i = 0; i < 5; i++) {
                        while (id != token%N) lock.wait();

                        System.out.println(id + " " + i);

                        token++;
                        lock.notifyAll();
                    }
                }
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }

    void start() {
        for (int i = 0; i < N; i++) {
            new Worker(i).start();
        }
    }

    public static void main(String[] args) {
        new TickTock().start();
    }
}
Improve this answer

5 Comments

I thought when the thread enters the synchronized (a.class) the object is locked?
You lock the class object in that statement.
I am stuck!!! but the calling thread is the one that locks the class object. Then I release the lock using notify() and wait().
Concurrency is very complicated, and there are alot of traps in the tools provided in Java. I would suggest reading amazon.com/Java-Concurrency-Practice-Brian-Goetz/dp/0321349601
Added a solution. Didn't really think it through so any bugs are left as excercises for the interested reader ;)
2

When comparing strings (and objects in general), you should use equals as opposed to == (which is generally reserved for primitives): while(time.equals("Tock")). == on strings will often times result in false when you want it to (and think it should) return true, and hence your loop will exit before expected.

Improve this answer

1 Comment

It's even worse because time is an instance variable and there are two instances so the check will not be true anyway. And that is lucky because you are only allowed to wait for objects that are locked and the threads are not locked. Keep on playing though. You will learn :)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.