0

As the title suggests, this is a Producer/Consumer implementation using Semaphores.

There are no deadlocking issues, but it seems whenever the Consumer looks to remove the next item in the queue, it is always the tail.

Pseudo-code:

class Consumer {
    while (!q.isEmpty() || !dataRead())
    {
       empty.acquire();
       mutex.acquire();
       queue.remove();
       mutex.release();
       full.release();
    }

    synchronized (caller)
    {
         caller.dataWritten = true;
         caller.notify();
    }
}

class Producer {
    for (int i=0; i < 100; i++)
    {
       full.acquire();
       mutex.acquire();
       queue.add(i);
       mutex.release();
       empty.release();
    }
    ...also notify Consumer that we're done...
}

class MainThread {
 ...
 new Thread(new Producer(args)).start();
 new Thread(new Consumer(args)).start();

 synchronized (this) {
    while (!dataWritten())
      wait();
 ...
 }

The output looks something like this:

  • Item 0 produced...value 0.
  • Item 1 produced...value 1.
  • Item 0 consumed...value 1.
  • Item 1 consumed...value 1.

Obviously, as I'm removing the head, I am expecting removal to be in the same order (FIFO).

I am using a ConcurrentLinkedQueue implementation, but have tried many other implementations like LinkedList, BlockingQueue implementations, etc.

Also, I have attempted to synchronize on the queue object and make the variable volatile as well in ditch efforts, but it did not make a difference.

I am thinking it has to do with a stale reference to the queue, but have run out of ideas to try.

EDIT: What is wrong with the output is that when Item 0 was consumed, it should have consumed value 0, not 1, because that was what was produced for Item 0.

I can't seem to reproduce using minimal code.

It seems if the data comes from an Iterator that takes a small amount of time to read information, the output is off (like in my example), but if I just shove some constant values in, the output is correct. Since there is only 1 Producer, why should this make a difference?

ryuu9187
  • 1
  • 1
  • 1
    Why give us pseduo-code? Give us an [MCVE](http://stackoverflow.com/help/mcve) instead. – Duncan Jones Jun 23 '14 at 13:42
  • 2
    Why is it that in the `Comsumer` you aquire `empty` but release `full`? Also, if you are using a `ConcurrentLinkedQueue` why are you using semaphores? – John B Jun 23 '14 at 13:42
  • Most likely there is a bug in your code, but without a self contained example we can run, it would be impossible to say where. – Peter Lawrey Jun 23 '14 at 13:49
  • @Duncan/Peter I will post a link to the MCVE. @John B If you need a reference to how Semaphores work, you can check out basic Semaphore use on wikipedia. empty.acquire checks if there is room in the buffer, full.release lets the producer know there is space...etc. – ryuu9187 Jun 23 '14 at 14:24
  • So, to verify, nothing apparently wrong with using any queue implementation across multiple threads as long as mutual exclusion is guaranteed? – ryuu9187 Jun 23 '14 at 15:30

1 Answers1

0

I am not sure, if I interpret the output of your program correctly. I assume that Item x refers to the integer you increment in the for-loop of the producer. How do you set the value of the value-field?

Since you first add item 0 to the queue and item 0 is the first one you consume, the behavior of the queue seems correct to me.

Roland
  • 489
  • 5
  • 14
  • The output is saying the first item consumed has a value of 1, when it does not. Item 0 has a value of 0. – ryuu9187 Jun 23 '14 at 17:23