CODEWITHSUNDEEP
javafifoconsumerproducer
Repi
Repi

Reputation: 1

Producer/Consumer FIFO Semaphore

so, I have been working on a producer/consumer problem, where I have 10 producers and 10 consumers. The consumers retrieve a number from this FIFO that was produced by a producer. All the consumers and producers are supposed to be working at the same time (they are threads). Now, I am not very good with semaphores at the moment, which is why I was trying to solve this problem. I can't get this to work no matter what.

semPop is a semaphore for the pop operation. semPush is a semaphore for the push operation. semWorkPush is a semaphore that prevents multiple threads from working at the same time in an push operation. semWorkPop does the same as semWorkPush but for the pop operation.

Also, open to suggestions in the way I write code so that it becomes easier to read! Thanks

public class SharedFifo {
    private Integer[] memory;
    private Integer[] ids;
    private Semaphore semPop = new Semaphore();
    private Semaphore semPush = new Semaphore();
    private Semaphore semWorkPush = new Semaphore();
    private Semaphore semWorkPop = new Semaphore();
    private int numberOfElements = 0;
    private int totalSize = 0;
    private int nextMemberToPop = 0;
    private int tail = 0;
    private int flag = 0;

    public SharedFifo(int a) {
        semPop.up();
        semPush.up();
        semWorkPush.up();
        semWorkPop.up();
        this.totalSize = a;
        this.memory = new Integer[a];
        this.ids = new Integer[a];
    }

    public void pushVal(int val, int id) {
        if (numberOfElements == totalSize) {
            semPush.down();
            semPop.up();
        }
        semWorkPush.down();
        numberOfElements++;
        memory[tail] = val;
        ids[tail] = id;
        this.tail = (tail + 1) % (this.totalSize);
        if(flag > 0) {
            System.out.println("flag is bigger than 0");
            semPop.up();
            this.flag--;
        }
        semWorkPush.up();
    }

    public Integer[] popVal() {
        Integer[] valAndId = new Integer[2];
        if (numberOfElements == 0) {
            semPop.down();  
            semPush.up();
            this.flag++;
        }
        semWorkPop.down();
        valAndId[0] = memory[nextMemberToPop];
        valAndId[1] = ids[nextMemberToPop];
        this.nextMemberToPop = (this.nextMemberToPop + 1) % (this.totalSize);
        numberOfElements--;
        semPush.up();
        semWorkPop.up();
        return valAndId;
    }
}

Upvotes: 0

Views: 705

Answers (1)

markspace
markspace

Reputation: 11030

The way you're going about this is so convoluted that I find it incomprehensible. Sorry to say that, but it's true. You have so many flags and ways of counting the buffer that I'm not surprised you have bugs. You're probably just making math errors.

All you need is two numbers. Normally a count and a head pointer. Three numbers will make it easier, head, tail and count. You need the count if you allow a head and tail to be the same index when the buffer is full. (I'm talking about a circular buffer here, which is what I think you're trying to implement.)

Here's some basic math equations for accessing a circular buffer. I'm using primitives here on purpose, just wait() and notify(), real code should use one of the classes from java.util.concurrent.

class CircularBuffer {
   private final int[] buffer = new int[20];
   private int head;
   private int count;
   public synchronized void put( int i ) throws InterruptedException {
      while( count == buffer.length ) wait();// full
      buffer[head++] = i;
      head %= buffer.length;
      count++;
      notifyAll();
   }
   public synchronized int get() throws InterruptedException {
      while( count == 0 ) wait(); // empty
      int tail = (head - count) % buffer.length;
      tail = (tail < 0) ? tail + buffer.length : tail;
      int retval = buffer[tail];
      count--;
      notifyAll();
      return retval;
   }
}

See how much smaller (and simpler) this code is? This is all you need.

Upvotes: 0

Related Questions