Setting and Getting Methods with wait() and notifyAll()

Question

I don't know why my get method doesn't work. It returns " ".

I have Producer and Consumer classes that use this class and Buffer interface that just have set and get methods. Producer reads from file and Consumer writes into another file. Both Producer and Consumer uses thread.

Please help me. Thanks in advance.

import java.util.Stack;

public class synchronizedFile implements Buffer {

public Stack<String> StackBuffer = new Stack<String>();

public void set(String value) {

    synchronized (StackBuffer) {
        if (StackBuffer.size() <= 15) {
            StackBuffer.push(value);
            System.out.println(StackBuffer.toString());
            StackBuffer.notifyAll();
            System.out.println("Consumer notify");
        } else {
            try {

                System.out.println("Produser is waitting--------------------------------");
                StackBuffer.wait();
                System.out.println("Consumer tries to write");
                set(value);

            } catch (Exception e) {
                e.printStackTrace();
            }
        }
    }
}

public String get() throws InterruptedException {

    String Flag = " ";
    synchronized (StackBuffer) {
        if (!StackBuffer.isEmpty()) {

            Flag = StackBuffer.firstElement();
            StackBuffer.remove(StackBuffer.firstElement());
            StackBuffer.notifyAll();
            System.out.println("Producer notify");
            return Flag;
        } else {
            StackBuffer.wait();
            System.out.println("Consumer is waitting --------------------");
            get();

        }

    }

    return Flag;

}

}

You should always call wait inside a loop that tests the condition you are waiting for. Read the documentation... — assylias, Feb 15 '13 at 13:59

score 1 · Answer 1 · answered Feb 15 '13 at 13:50

You are at least missing a Flag = get() in your else branch of the get() method.

Apart from that consider using a BlockingQueue, there are implementations in java.util.concurrent that do the heavy lifting in concurrent programming for you. Using the low level constructs wait and notify is error prone. Without checking it thoroughly, I would be surprised if your implementation would be correct.

score 1 · Answer 2 · edited May 23 '17 at 10:32

1

You wait and notify for two different status: full and empty. Therefore, you must use two separate lock objects. This has been covered a short while ago here.

Basically, if you are not implementing this for the sake of using the Stack class or some kind of assignment, use a BlockingQueue from java.util.concurrent.

edited May 23 '17 at 10:32

Community

1
1

answered Feb 15 '13 at 13:54

Ralf H

1,392
1
9
17

score 0 · Accepted Answer · answered Feb 15 '13 at 13:50

0

Here you call get() recursively but throw away its result:

StackBuffer.wait();
System.out.println("Consumer is waitting --------------------");
get();

Something like Flag = get(); or return get();would be more appropriate.

(Also I'm not sure if entering synchronized section twice with doing wait is valid. Maybe it is, I'm just unsure).

answered Feb 15 '13 at 13:50

Anton Kovalenko

20,999
2
37
69

synchronized is reentrant so there is no issue with calling set from set or get from get. – assylias Feb 15 '13 at 13:52

Setting and Getting Methods with wait() and notifyAll()

3 Answers3