3

I've this class:

public class MyThread implements Runnable {

    private static boolean canAccess = true;
    private Thread t;

    public FirstThread(String name) {
    t = new Thread(this);
    t.setName(name);
    }

    public void start() {
        t.start();
    }

    private synchronized void accessed(String name) throws InterruptedException {
    if (canAccess) {
        canAccess = false;
        System.out.println("Accessed " + name);
        try {
        Thread.sleep(5000);
        } catch (Exception e) {
        }
        canAccess = true;
        System.out.println("NOTIFY: " + name);
        notifyAll();
    }
    System.out.println("WAIT: " + name);
    wait();

    }

    @Override
    public void run() {
    while (true) {
        try {
        accessed(Thread.currentThread().getName());
        } catch (InterruptedException e) {
        e.printStackTrace();
        }
    }
    }
}

And this is my output:

Accessed 1
WAIT: 3
WAIT: 5
WAIT: 7
WAIT: 9
WAIT: 0
WAIT: 2
WAIT: 4
WAIT: 6
WAIT: 8
NOTIFY: 1
WAIT: 1

and my app freeze (deadlock state). Seems that the notifyAll method doesn't work. Where is my error?

My Main class.

public class Main {

    public static void main(String[] args) {
    MyThread [] threads = new MyThread[10];
    for(int i=0;i<threads.length;i++) {
        threads[i] = new MyThread(""+i);
        threads[i].start();
    }

    }

}
CeccoCQ
  • 3,746
  • 12
  • 50
  • 81
  • 1
    Starting a thread on this in the constructor is rarely a good idea. Not sure if that is the issue. See for example: http://stackoverflow.com/questions/5623285/java-why-not-to-start-a-thread-in-the-constructor-how-to-terminate – assylias Jul 08 '12 at 14:56
  • You're right, but I've changed my code and still not work. – CeccoCQ Jul 08 '12 at 14:59
  • 1
    I don't see the point is setting `canAccess` first to `false` and then to `true` within the only `synchronized` block that uses this var. – Marko Topolnik Jul 08 '12 at 14:59
  • Am I correct that your create 10 instances of class MyThread and then call start() of every method. Could you provide the starting piece of code this will show a better picture. I guess every thread waits on it's own instance and nobody to awaken them. – Alexey A. Jul 08 '12 at 15:02
  • @MarkoTopolnik Sorry, I didn't understand . If I set canAccess = true and call notifyAll, theorically I should have the "Thread Race" or I've missing something? – CeccoCQ Jul 08 '12 at 15:02
  • I suppose FirstThread is a typo and you meant MyThread? Also, are you aware that sleep won't release the lock on the class? – Miquel Jul 08 '12 at 15:02
  • Why do you set `canAccess` to `false` earlier? That won't be observed anywhere. – Marko Topolnik Jul 08 '12 at 15:04
  • With "Sleep" I would to simulate the thread that uses shared resource. – CeccoCQ Jul 08 '12 at 15:06
  • No other thread can even **enter** the `accessed` method, let alone **check the state of the `canAccess` flag**, while another thread is exeucting that `sleep`. – Marko Topolnik Jul 08 '12 at 15:10

2 Answers2

2

wait means that the thread releases the lock and goes into a dormant state until another thread notifies it. notifyAll means that the thread tells all the other threads waiting on the lock being used in the current synchronized block to wake up and try to acquire the lock again. Your code example doesn't have any cases where multiple threads are trying to acquire the same lock so using wait and notifyAll here doesn't make any sense. There's nothing to wake up the thread once it calls wait.

One typical use of wait and notify: You might have many producers putting stuff in a queue, and consumer threads that take stuff out of the queue. The queue has a take method that the consumer calls, if the queue is empty then it calls wait and the consumer blocks. The queue has a put method that calls notifyAll when something goes into the queue so that any waiting consumer threads wake up.

There's a producer-consumer example of using wait and notifyAll in the Java tutorial.

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
1

Every Thread waits on it's own instance, that's why they all are stuck in one place.

If you had a private static Object LOCK = new Object(); and call LOCK.wait(); and LOCK.notify(); this could be another story.

I have also doubts about synchronized modifier for accessed() method. It's just doesn't have use in the described situation. I would better modify the "canAccess" variable in synchronized block.

Alexey A.
  • 1,389
  • 1
  • 11
  • 20