java assignment synchronization producer consumer

Question

I've written a Multithreading code for producer consumer problem in which I've written synchronized block inside the run method of consumer and producer thread which takes lock on shared list(I assumed) So the point of question is that, will there be locking on the list, because as per each thread will have their own synchronized block but they are sharing the same list instance

public class Main {
    static boolean finishFlag=false;
    final int queueSize = 20;
    List<Integer> queue = new LinkedList<>();
    Semaphore semaphoreForList = new Semaphore(queueSize);

    public Main(int producerCount,int consumerCount) {
        while(producerCount!=0) {
            new MyProducer(queue,semaphoreForList,queueSize).start(); //produces the producer
            producerCount--;
        }
        while(consumerCount!=0) {
            new MyConsumer(queue,semaphoreForList,queueSize).start(); //produces the consumer
            consumerCount--;
        }
    }

    public static void main(String args[]) {
        /*
         * input is from command line 1st i/p is number of producer and 2nd i/p is number of consumer
         */
        try {
            Main newMain = new Main(Integer.parseInt(args[0]),Integer.parseInt(args[1]));
            try {
                Thread.sleep(30000);
            }
            catch(InterruptedException e) {
            }
            System.out.println("exit");
            finishFlag=true;
        }
        catch(NumberFormatException e) {
            System.out.println(e.getMessage());
        }

    }
}

class MyProducer extends Thread{
    private List<Integer> queue;
    Semaphore semaphoreForList;
    int queueSize;
    public MyProducer(List<Integer> queue, Semaphore semaphoreForList,int queueSize) {
        this.queue = queue;
        this.semaphoreForList = semaphoreForList;
        this.queueSize = queueSize;
    }
    public void run() {
        while(!Main.finishFlag) {
            try {
                Thread.sleep((int)(Math.random()*1000));
            }
            catch(InterruptedException e) {
            }
            try {
                if(semaphoreForList.availablePermits()==0) {//check if any space is left on queue to put the int
                        System.out.println("no more spaces left");
                }
                else {
                    synchronized(queue) {
                        semaphoreForList.acquire(); //acquire resource by putting int on the queue
                        int rand=(int)(Math.random()*10+1);
                        queue.add(rand);
                        System.out.println(rand+" was put on queue and now length is "+(queueSize-semaphoreForList.availablePermits()));        
                    }
                }
            }
            catch(InterruptedException m) {
                System.out.println(m);
            }
        }
    }   
}

public class MyConsumer extends Thread{
    private List<Integer> queue; //shared queue by consumer and producer
    Semaphore semaphoreForList;
    int queueSize;
    public MyConsumer(List<Integer> queue, Semaphore semaphoreForList,int queueSize) {
        this.queue = queue;
        this.semaphoreForList = semaphoreForList;
        this.queueSize = queueSize;
    }
    public void run() {
        while(!Main.finishFlag) {//runs until finish flag is set to false by main
            try {
                Thread.sleep((int)(Math.random()*1000));//sleeps for random amount of time
            }
            catch(InterruptedException e) {
            }
            if((20-semaphoreForList.availablePermits())==0) {//checking if any int can be pulled from queue
                System.out.println("no int on queue");
            }
            else {
                synchronized(queue) {
                    int input=queue.remove(0);//releases the resource(position in queue) by pulling the int out of the queue and computing factorial 
                    semaphoreForList.release();
                    int copyOfInput=input;
                    int fact=1;
                    while(copyOfInput!=0) {
                        fact = fact*copyOfInput;
                        copyOfInput--;
                    }
                    System.out.println(input+" was pulled out from queue and the computed factorial is "+fact+
                            " the remaining length of queue is "+(queueSize-semaphoreForList.availablePermits()));
                }   
            }       
        }
    }
}

Answer 1

I would rather recommend to use the java.lang.Object methods wait() and notify() to create a consumer-producer algorithm. Using this approach the queue won't be blocked by endlessly repeating and unnecessary synchronized statements which I think is a more performant and "event driven" solution.

This link might be helpful - https://www.geeksforgeeks.org/producer-consumer-solution-using-threads-java/

Answer 2

Yes, the mutex/monitor is associated with the Java Object instance, which is the shared list in this instance. Which means all threads lock same mutex (associated with queue, and are synchronized through this.

So the good part: You program is actually thread-safe.

However the additional semaphore actually doesn't make a lot of sense in a variety of ways:

• The checks (e.g. for availablePermits) happen outside of the lock, and are therefore only a best-guess about the state of your queue. It could be different shortly afterwards.
• Trying to acquire a semaphore inside a lock, which can only be released inside the same lock, looks like a guaranteed recipe for a deadlock.

As AnDus has mentioned, this could probably be better solved via using the wait and notify methods which act as a condition variable. Most likely you need even two, one to unblock producers and one to unblock consumers.

In general, if this is not a coding exercise, use a class which already implements your desired functionality. In this case, java.util.concurrent.BlockingQueue seems like what you want.