0

I have created my own queue .

Queue.java

public class MyQueue {

    private int size;

    private Queue<String> q;

    public MyQueue(int size ,Queue<String> queue) {
    this.size = size;
    this.q = queue;

    }

    //getter and setter    

    public synchronized void putTail(String s) {

System.out.println(this.size); // It should print 0, 1,2

          while (q.size() != size) {

             try {
                wait();
             }
             catch (InterruptedException e) {
             }
          }
          Date d = new Date();

          q.add(d.toString());

          notifyAll();

       }

}

MyProducer.java

import com.conpro.MyQueue;

public class MyProducer  implements Runnable {

    private final MyQueue queue;

    private final int size; 

    MyProducer(int size,MyQueue q) { this.queue = q; this.size = size; }


    @Override
    public void run() 
    {
        queue.putTail(String.valueOf(Math.random()));
    }

}

MyTest.java

public class MyTest {

    public static void main(String[] args) {

        Queue q = new PriorityQueue<String>();
        MyQueue mq = new MyQueue(3,q);

         MyProducer p = new MyProducer(3,mq);
         MyProducer p1 = new MyProducer(3,mq);
         MyProducer p2 = new MyProducer(3,mq);
         new Thread(p).start();
         new Thread(p1).start();
         new Thread(p2).start();

    }

}

Now Here I have created 3 producer . So after executing this 3 lines , queue should be full.

Output should be :

0
1
2

But it is only printing 0.

Why?

P.S : I have written only producer code since I have not reached there yet.

Thinker
  • 6,820
  • 9
  • 27
  • 54

3 Answers3

1

Since putTail() is synchronized, only one of the three threads can enter it. That thread then sits forever inside the while (q.size() != size) loop, while the other two threads remain unable to enter the method.

NPE
  • 486,780
  • 108
  • 951
  • 1,012
  • Am I going in right direction to solve a consumer producer problem? What I can change? – Thinker Aug 10 '13 at 19:46
  • @Thinker: TBH, I don't think you are. The following might be a good start: http://stackoverflow.com/questions/2332537/producer-consumer-threads-using-a-queue – NPE Aug 10 '13 at 19:47
  • What I can change in putTail method? – Thinker Aug 10 '13 at 19:47
  • Is this really correct? `wait()` releases the monitor on `this` and allows other objects to synchronize on it, so really what I think is happening is that all three threads end up waiting on the `wait()` call. – Brian Aug 10 '13 at 19:48
  • I tried debugging. I guess all 3 threads are stuck at wait(). what i should do? – Thinker Aug 10 '13 at 19:53
  • @Oil I waited for long time. And now it print 0 0 0 – Thinker Aug 10 '13 at 19:54
  • And when i changed the condition to `q.size() == size` .and now it is printing 0 1 2 – Thinker Aug 10 '13 at 19:56
  • @OliCharlesworth and others, see my answer. The `wait` releases the monitor which allows the other threads to enter the method, but the issue is that all the threads enter the while loop and end up waiting without ever being notified. They shouldn't enter the while loop unless they're supposed to (i.e. the queue is full), so that's why changing his condition worked. – Brian Aug 10 '13 at 20:53
1

The problem is that all 3 threads enter the wait() but never get notified via notifyAll

There's problem with your code that doesn't really make it a blocking queue. Here's what I would expect a blocking queue to do:

  1. Queue is bound to maximum size of 3, start with no elements
  2. Get something from a producer, size is not yet 3, add it, don't block
  3. Get something else from a producer, size is not yet 3, add it, don't block
  4. Get something else from a producer, size is not yet 3, add it, don't block
  5. Get something else from a producer, size is now 3, block until something is taken
  6. Get something else from a producer, size is still 3, block until something is taken
  7. A consumer takes from the queue, the threads from (5) and (6) are notified, and the first one to get scheduled obtains the lock long enough to add his element, the other one is forced to block again until another consumer takes from the queue.

Here's what yours is actually doing as-written:

  1. Queue is bound to maximum size of 3, start with no elements
  2. Get something from a producer, size is not yet 3, block on wait() without adding it
  3. Get something from a producer, size is not yet 3, block on wait() without adding it
  4. Get something from a producer, size is not yet 3, block on wait() without adding it

In all 3 cases of adding the element, the element doesn't actually get added, and we get stuck at wait() because all 3 enter the while loop, then nothing ever calls notifyAll().

Your fix in the comments:

while (q.size() == size)

This makes it do what it's supposed to: if the size has already reached the maximum, block until it's told to continue via a notify, then check if the size is still the maximum. For the thread from my example above that receives the lock after being notified (e.g. the thread from step 6), it will get the chance to add its message. The thread that doesn't receive the lock will receive the lock after the first one releases it, but the size will have increased to the max size again, which causes it to block again. That being said, I think your approach is a good start.

The one thing in your code that's incorrect is that you're calling notifyAll after you add it. Adding will never cause the queue size to shrink, but you're notifying all the threads waiting in the putTail method to continue. There's no reason to notify the threads that are waiting to add something to the queue if you just put something into it that made it reach the maximum size anyway. I think you meant for that notify to do something with the threads waiting on your eventual take method, which leads me to my next point:

Your next step will be to have two lock objects instead of always using this. That way the take method can block separately from the put method. Use one lock to wait in the put method and notifyAll on it in take, then use the other lock to wait in the take method and notifyAll on it in put. This will make it so that you can separately notify the takers and putters without notifying all of them at once like you would using this.notifyAll.

Brian
  • 17,079
  • 6
  • 43
  • 66
0

The problem is in MyQueue class's putTail() method.There you are calling wait() on this (current object),and it never be notified.Then the thread will wait forever.

Prabhaker A
  • 8,317
  • 1
  • 18
  • 24