How to synchronize shared variable with two Semaphores?

Question

I have a exercise where I have a Feast where Person from persons N = 10 eat 1 amount of servings from pot at a time. Pot have maximum amount of servings M = 5. There is also a Cook who fill the pot when it is empty servingsAvailable = 0. Person can't eat during filling. I have to synchronize the threads only chaning the methods fill and getServings from Pot class (these methods were empty at the beginning).

Can you tell me what am I doing wrong in this code? Total amount should be 1000 but it is always less. I achieve situation where pot is filling then 5 persons eat, then its filling etc. but the number of servings eaten is inconsistent.

Person class

public class Person extends Thread { // Reprezentuje tubylca
    Pot pot;
    int servingsConsumed = 0;
    public Person(String name, Pot pot) {
        super(name);
        this.pot = pot;
    }
    @Override
    public void run() {
        try {
            for (int i = 0; i < 100; ++i) {
                pot.getServing(this.getName());
                ++servingsConsumed;
                Thread.yield();
            }
        } catch(InterruptedException e) {
            return ;
        }
    }
}

Cook class

public class Cook extends Thread { // Reprezentuje kucharza
    Pot pot;
    public Cook(Pot pot) {
        this.pot = pot;
        setDaemon(true);
    }
    @Override
    public void run() {
        try {
            while(!isInterrupted()) {
                pot.fill();
            }
        } catch(InterruptedException e) {
            return ;
        }
    }
}

Pot.class

import java.util.concurrent.Semaphore;

public class Pot {
    static final int M = 5; // Pojemność kotła
    private Semaphore emptyPot = new Semaphore(1);
    private Semaphore available = new Semaphore(0);
    private int servingsAvailable = 0;
    private int totalServedCount = 0;

    private synchronized void insertServings(int value) {
        servingsAvailable = value;
    }

    private synchronized int removeServing() {
        --servingsAvailable;
        ++totalServedCount;
        return servingsAvailable;
    }

    public int getTotalServedCount() {
        return totalServedCount;
    }

    public void getServing(String nameOfPerson) throws InterruptedException {
        available.acquire();
        if (servingsAvailable != 0) {
            removeServing();
            System.out.println(nameOfPerson + " ate 1 portion from pot");
        }
        available.release();
    }

    public void fill() throws InterruptedException {
        available.acquire();
        if (servingsAvailable == 0) {
            insertServings(M);
            System.out.println("Fill the pot with M = " + M);
        }
        available.release();
    }
}

Feast class (main)

public class Feast {
    public static void main(String[] args) throws InterruptedException {
        Pot pot = new Pot();
        Cook cook = new Cook(pot);
        final int N = 10;
        Person[] people = new Person[N];
        for (int i = 0; i < people.length; ++i) {
            people[i] = new Person("Person " + i, pot);
        }
        cook.start();
        for (Thread t : people) {
            t.start();
        }
        for (Thread t : people) {
            t.join();
        }
        cook.interrupt();
        System.out.printf("Total served: %d.\n", pot.getTotalServedCount());
        for (Person p : people) {
            System.out.printf("[%s] Ate %d servings.\n", p.getName(), p.servingsConsumed);
        }
        System.out.println("Finishing simulation.");
    }
}

And the result I achieve so far: I think it should show 1000 instead of 245 here.

Answer 1

You're using your semaphores like a simple mutex, without any way for callers to know how many servings are available. If you want to signal the state of the pot, you should be updating them as the servings get filled and consumed:

public void getServing(String nameOfPerson) throws InterruptedException {
    // take a permit and keep it
    available.acquire();
    System.out.println(nameOfPerson + " ate 1 portion from pot");
    if (removeServing() == 0) {
        // release a refill permit to the Cook
        emptyPot.release();
    }
}

public void fill() throws InterruptedException {
    // wait till pot is empty
    emptyPot.acquire();
    insertServings(M);
    System.out.println("Fill the pot with M = " + M);
    // release a permit for each serving
    available.release(M);
}