Take the 2-minute tour ×

So this is my first foray into threads and thus far it is driving me insane. My problem seems to be some kind of synchronization error that causes my consumer thread to hang. I've looked at other code and just about everything I could find and I can't find what my error is. There also seems to be a discrepancy between the code being executed in Eclipse and via javac in the command line.

Intention - Using a bounded buffer (with 1000 slots) create and consume 1,000,000 doubles. Use only notify and wait.

Problem - In Eclipse the consumer thread will occasionally hang around 940,000 iterations, but other times completes. In the command line the consumer thread always hangs.

share|improve this question
I see you have tried '//if(i>999000)'. Does this indicate that the consumer thread has not finished because one or more queue dobjects have got 'lost', or is the consumer stuck for some other reason? You may like to try queueing up integers instead and checking in the consumer that every value received, (except the first), is one more than the last, and stopping/excepting immediately if they are not. Random doubles are just about the worst thing you could have chosen to queue because of the difficulty of checking/comparing them in the consumer. –  Martin James Apr 11 '12 at 9:08
add comment

4 Answers

Wait() method can interrupt in spurious way (that is, without notification), refer here. So you need to replace all your if (condition) { wait(); } to while (condition) { wait(); }. Maybe this is the cause.

share|improve this answer
It might be better to stop using OS that 'support' spurious wakeups :) –  Martin James Apr 11 '12 at 8:28
Which OSs support? I'm not aware of this, so it's interesting :) –  Alexey Berezkin Apr 11 '12 at 8:53
AFAIK, only U**x-like OS support this feature. It is penguin-specific <g> –  Martin James Apr 11 '12 at 8:57
add comment

You messed with addPlace and getPlace. For the sake of clarity, I'll rename them nextWrite and nextRead. So this is your add()

if ((nextWrite + 1) == nextRead) {
buff[nextWrite] = someRandomNumber;
nextWrite = (nextWrite + 1) % 1000;

And this is your get()

if (nextRead == nextWrite) {

The logical error is obvious: since nextWrite is in the range [0; 999], nextWrite + 1 will be in [1; 1000], but nextRead can only be in [0; 999]. Everytime nextWrite is 999 or nextRead is 0, the wait call will never be executed and the Producer may overwrite data that has not been read yet.

The producer may stop overwriting at some point, but effectively on an imaginary multicore machine where one core is a million times faster than the other one, the Producer will complete its run() and will terminate, because it only stops when nextWrite + 1 == nextRead.

On our imaginary machine, the Consumer will hang as soon as nextRead becomes 0 (thus equal to nextWrite, which the Producer last set to 0 because it runs exactly a million iteration and your buffer counter is defined as i % 1000), because at that moment it will wait() for a notification by the Producer, but since it has terminated, no notification will ever arrive.

This is an improved (and working) version of your code

EDIT I just forgot the (trivial) solution:

public synchronized void add(double randomNumber) throws InterruptedException {
    if((nextWrite + 1) % 1000 == nextRead)
    buff[nextWrite] = randomNumber;
    nextWrite = (nextWrite+1)%1000;
share|improve this answer
+1 Looks good to me. I'm surprised I didn't notice the failure to wrap the index when checking for buffer full. It's a pity the OP did not simply post incrementing integers and check them in the consumer - the off-by-one issue would have been obvious. –  Martin James Apr 11 '12 at 12:42
Sure, printing integers he would have discovered it soon. And maybe he wouldn't have posted here :P I didn't notice it too, until I refactored his code with meaningful names and the logical error immediately came up –  Raffaele Apr 11 '12 at 12:49
Thank you very much for your help however I think this has now reversed the issue. It now hangs intermittently at 999,999 in the Producer thread. The consumer thread terminates which shouldn't be possible if Producer is at 999,999. –  user972425 Apr 11 '12 at 16:13
@user972425 the if preambles in add and get should be while(<condition>) {wait();} –  Raffaele Apr 12 '12 at 8:11
add comment

Some points:

  • In Java generally the first class name char has to be uppercase: so call your class Producer, Consumer and Buffer.
  • When you say that you try this on Eclipse / Command Line it means that you run producer in Eclipse and Consumer in command line or vice versa? If so it cannot works, buffer is not shared between the two processes.
share|improve this answer
Just read his main –  Raffaele Apr 11 '12 at 12:07
add comment

Well, I think I see logical problem in your program. In standart Produce-Consume routine, you can Produce as much as you want, but you can Consume only if there is something produced.

In your implementation, you Produce, wait till its Consumed, ant then Produce again.

I think its not what you intended to do.

Remove wait() block from add method, and remove notify() from get() method. Also, I don't think threads join() is necessary in main.

Edit: and your get() method always goes into wait(). Even if Producer already finished its job. Think about it. If Producer already finished its job, you wont get notify().

share|improve this answer
add comment

Your Answer


