Problem with Consumer/Producer implementation

[Anton Shaikin]

First of all, I have to admit that I posted this question on Sun Forums earlier. But since Oracle acquisition of Sun, forums seem to be dead, and there's a little chance to get a response there. Every time I log in, I see "0 Users online". So, I do respect JavaRanch community, and therefore apologize for having to copy-paste my original post. Thank you for understanding.

I spent considerable amount of time today trying to figure out what I'm doing wrong to no avail. Sorry for such long code, but I see no way to reduce it. And also, please, do not point at design flaws - I do know that from the point of OO it's not semantically correct to declare Producer and Consumer as inner classes of a single class and such. It doesn't matter here. I'm just trying to understand why it doesn't work. So, in first call to wait Consumer releases lock on prod object. Then Producer thread enters synchronized block (acquires a lock on the prod object). However, after call to notify and exiting synchronized block (therefore releasing a lock), it doesn't wake the Consumer up for no apparent reason (well, in fact it does occasionally, once per 10 loops). Moreover, when I uncomment a line right after Consumer synchronized block, which outputs "Producer should have released lock by now" to console, the behavior changes a bit. Consumer starts to consume values, but again, asynchronously and in chaotic manner. Could anybody clarify what is wrong in this code? Any help would be highly appreciated.
P.S. Also, don't worry about main method inside nested class. After reading Bruce Eckel's "Thinking in Java" I figured it's really neat to have main, designed for testing purposes only, inside a nested class, which you could physically remove from your hard drive after testing is done. And I do invoke main correctly as following: java threads.WaitDemo$Tester.
So here is the code:

  package threads;
 
public class WaitDemo {
 
  private class Producer extends Thread {
    int i;
		
    public void run() {
      System.out.println("Producer gets some sleep before going to work");
      try {
        Thread.sleep(500);
      } catch(InterruptedException e) {
          e.printStackTrace();
	}
      for(int i = 0; i < 20; i++) {
        synchronized(this) {
          System.out.println("Producer enters synchronized block");
          try {
            Thread.sleep(500);
          } catch(InterruptedException e) {
            e.printStackTrace();
          }
					
          this.i = i;
          System.out.println(Thread.currentThread().getName() + " produced " + this.i);
          notify();
        }
        //System.out.println("Producer should have released lock by now");
      }
    }
  }
	
  private class Consumer implements Runnable {
    public void run() {
      Producer prod = new Producer();
      prod.setName("Producer");
      prod.start();
      while(true) {
        System.out.println("Consumer enters while loop");
        synchronized(prod) {
          System.out.println("Consumer enters synchronized block");
          try {
            prod.wait();
          } catch(InterruptedException e) {
            e.printStackTrace();
          }
	  Thread t = Thread.currentThread();		
          System.out.println(t.getName() + " consumed: " + prod.i);
        }
      }
    }
  }
	
  private static class Tester {
    public static void main(String[] args) {
      new Thread(new WaitDemo().new Consumer(), "Consumer").start();
    }
  }
 
}
  

[amitabh mehra]

1) notify() does not guarantee that consumer thread will be the next thread to run. It only guarantees that consumer thread is now *eligible to run*, but, since now both threads - Producer thread and Consumer thread are *eligible to run*, it all depends on the Thread Scheduler to schedule which thread should go next.

2) Consumer thread is not consuming in chaotic manner.

System.out.println(t.getName() + " consumed: " + prod.i);

You are printing the prod.i value. This will be the most recent one.

Better option to implement a Producer/Consumer problem would be to go for BlockingQueues else have some shared DataStructure in which Producer can go on inserting and consumer can pick items from there.

[Anton Shaikin]

OK. Thank you. The problem is now solved.