Producer-Consumer Semaphore Implementation

Question

I have implemented a Producer-Consumer by Semaphore implementation.

I am looking forward to for your code review comments.

Thanks.

package ProducerConsumer.Semaphore;

import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Semaphore;

public class SemaphoreMain {

    public static void main(String args[]){

        List<Integer> arrayList = new LinkedList<>();
        int capacity = 50;

        Semaphore semaphoreProducer = new Semaphore(1);
        Semaphore semaphoreConsumer = new Semaphore(0);

        Producer producer = new Producer(arrayList, capacity, semaphoreProducer, semaphoreConsumer);
        Consumer consumer = new Consumer(arrayList, capacity, semaphoreProducer, semaphoreConsumer);

        Thread t1 = new Thread(producer);
        Thread t2 = new Thread(consumer);

        ExecutorService es = Executors.newFixedThreadPool(2);
        es.submit(t1);
        es.submit(t2);

        es.shutdown();

    }
}

Producer.java

package ProducerConsumer.Semaphore;

import java.util.List;
import java.util.concurrent.Semaphore;

public class Producer implements Runnable {

    List<Integer> linkedList;
    Semaphore semaphoreProducer;
    Semaphore semaphoreConsumer;
    int capacity;

    public Producer(List linkedList, int capacity, Semaphore semaphoreProducer, Semaphore semaphoreConsumer){
        this.linkedList = linkedList;
        this.capacity = capacity;
        this.semaphoreProducer = semaphoreProducer;
        this.semaphoreConsumer = semaphoreConsumer;
    }

    @Override
    public void run() {

        for (int i=0;i<50;i++){

                    try {
                        semaphoreProducer.acquire();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    linkedList.add(i);
                    System.out.println("Added" + i);
                    semaphoreConsumer.release();

        }

    }
}

Consumer.java

package ProducerConsumer.Semaphore;

import java.util.List;
import java.util.concurrent.Semaphore;

public class Consumer implements Runnable{

    List<Integer> linkedList;
    Semaphore semaphoreProducer;
    Semaphore semaphoreConsumer;

    public Consumer(List linkedList, int capacity, Semaphore semaphoreProducer, Semaphore semaphoreConsumer){
        this.linkedList = linkedList;
        this.semaphoreProducer = semaphoreProducer;
        this.semaphoreConsumer = semaphoreConsumer;
    }

    @Override
    public void run() {

        while(true){

                    try {
                        semaphoreConsumer.acquire();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    System.out.println("removed :" + linkedList.remove(0));
                    semaphoreProducer.release();


        }

    }
}

Answer 1

Capacity

You don't use capacity, yet you pass it into both your Producer/Consumer classes. Superficially, it looks like you should be passing the capacity in when creating your Producer semaphore. This would allow 'capacity' items to be put into your list. At the moment, you're restricting it to only putting a single item into the list, then reading it back out again.

Thread safety

As it stands, your program works, however, it only works because you have a single consumer and producer. The way you're using the semaphores means that either a consumer can be running, or the producer can be running, they can't both be running at the same time. As a consequence, only one thread is working with the list. If you start supporting more than one producer/consumer however you'll have the potential for having more threads using the collection concurrently... in which case you should consider alternate protection or using a concurrent collection, I'd suggest one of the Queues.

Naming

Some of your names are a little confusing / misleading...

List<Integer> arrayList = new LinkedList<>();

Creates a linked list that's called arrayList...

This is obviously subjective, but I'm also not a fan of semaphoreProducer... I can't help but expect it to be something that produces semaphores. producerSemaphore works better for me, but as I said...very subjective.

Exit Strategy

Your Consumer thread doesn't have a way to exit, it just keeps on running. Consider adding some way of notifying the consumer that it's time to shutdown so that the thread can be exited cleanly without you needing to force it to terminate. This will become more important if your consumer starts doing some processing that takes a while that you don't want to exit in the middle of...