Fast food program, multiple threads and semaphores Java

Question

So I've briefly stumbled across Semaphores in my programming journey and ever so recently took it upon myself to devise a program replicating that of a fast food chain.

I'm going to try explain as best I can, please let me know in the comment section if I need to be more thorough

The Plan:

Have a producer and a consumer (tills and worker) So the tills take the order(Place them in a buffer, so a circular array..) and the worker processes that order(Takes orders out of the circular array)

I'm trying to implement semaphores so that when an order has been made, that specific till cannot be used until the worker has processed the order. And also use semaphores so that the officer can only take out one order at a time

Here is the main method:

public class FastFood {

    /**
     * @param args the command line arguments
     */
    static Buffer buff = new Buffer(2);
    static Semaphore semWorker = new Semaphore(1);
    static Semaphore semTills = new Semaphore(1);
    static int totalOrders = 10;
    static int startOrders = 0;
    static int processedOrders = 0;

    public static void main(String[] args) {
        // TODO code application logic here

        int numberOfWorkers = 2;
        int numberOfTills = 3;
        int numberOfFoodChoices = 4;
        Random rand = new Random();

        Tills[] tills = new Tills[numberOfTills];
        Worker[] workers = new Worker[numberOfWorkers];

        //int tillId, int foodId, Buffer buff
        for (int i = 0; i < tills.length; i++) {
            int foodId = rand.nextInt(numberOfFoodChoices) + 1;
            tills[i] = new Tills(i, foodId, buff);
            tills[i].start();
        }

        //int workerId, Buffer buff
        for (int i = 0; i < workers.length; i++) {
            workers[i] = new Worker(i, buff);
            workers[i].start();
        }

        for (Tills till : tills) {
            try {
                till.join();
            }catch (InterruptedException ex) {
                System.out.println(ex);
            }
        }

        for (Worker worker : workers) {
            try {
                worker.join();
            }catch (InterruptedException ex) {
                System.out.println(ex);
            }
        }

So as you can see by the main method I am looping and running the array of threads for both the worker and the tills.

Here is the tills class. So this creates the order. You will be able to see that I am using FastFood.semTills.down() and FastFood.semTills.up() this is using the semaphore. So down is acquiring the semaphore and up is releasing it. However the problem is my logic for the positioning of these semaphore downs and ups.

public class Tills extends Thread {
    private final Buffer buff;
    private final int foodId;
    private final int tillId;

    public Tills(int tillId, int foodId, Buffer buff) {
        this.tillId = tillId;
        this.foodId = foodId;
        this.buff = buff;
    }

    @Override
    public void run(){
        FastFood.semTills.down();
        while(FastFood.startOrders < FastFood.totalOrders){
            FastFood.semTills.up();
            buff.acquire(); 
            while(buff.isFull()){
                try {
                    buff.release();
                    sleep(100);
                } catch (InterruptedException ex) {
                    Logger.getLogger(Tills.class.getName()).log(Level.SEVERE, null, ex);
                }
            }
            FastFood.startOrders++;
            Order v = new Order(foodId, tillId);
            System.out.println(v.toString());            
            try {
                Random n = new Random();
                int time = n.nextInt(100) + 1;
                buff.release();
                sleep(time);
                buff.insert(v);
            } catch (InterruptedException ex) {
                System.out.println(ex);
            }

        }
    }

The worker class is somewhat the same, yet I want to ensure that only one worker can process a specific order at a time ( It is fine to enable multiple workers to enable multiple orders)

public class Worker extends Thread{
    private final int workerId;
    private final Buffer buff;


    public Worker(int workerId, Buffer buff) {
        this.workerId = workerId;
        this.buff = buff;
    }

    public void run(){
        FastFood.semWorker.down();
        while(FastFood.totalOrders>FastFood.processedOrders){
            buff.acquire();
            while(buff.isEmpty()){
                FastFood.semWorker.up();
                try {
                    buff.release();
                    sleep(100);
                } catch (InterruptedException ex) {
                    Logger.getLogger(Worker.class.getName()).log(Level.SEVERE, null, ex);
                }
            }
            FastFood.processedOrders++;
            System.out.print("Worker: " + workerId);
            buff.remove();
            buff.release();
            try {
                Random n = new Random();
                int time = n.nextInt(100) + 1;
                sleep(time);
            } catch (InterruptedException ex) {
                System.out.println(ex);
            }

        }
        FastFood.semWorker.up();
    }

This is the output I'm getting, you can see that it is not waiting for the orders to be processed, thus the positioning of my semaphores must be wrong, I've tried all sorts of possibilities:

run:
FoodId: 3 TillId : 1 Order Count : 0
FoodId: 3 TillId : 0 Order Count : 1
FoodId: 4 TillId : 2 Order Count : 2
FoodId: 4 TillId : 2 Order Count : 3
FoodId: 4 TillId : 2 Order Count : 4
FoodId: 3 TillId : 0 Order Count : 5
FoodId: 3 TillId : 0 Order Count : 6
Worker: 1 Food: 3 TillId: 0
Worker: 0 Food: 3 TillId: 0
FoodId: 3 TillId : 0 Order Count : 7
FoodId: 3 TillId : 0 Order Count : 8
Worker: 1 Food: 3 TillId: 0
FoodId: 3 TillId : 0 Order Count : 9
FoodId: 3 TillId : 1 Order Count : 10
Worker: 0 Food: 3 TillId: 0
Worker: 1 Food: 3 TillId: 1
Worker: 0 Food: 3 TillId: 0
Worker: 1 Food: 4 TillId: 2
Worker: 0 Food: 3 TillId: 0
Worker: 1 Food: 4 TillId: 2
Worker: 0 Food: 3 TillId: 0
10

Quick Class Brief:

FastFood: Main, creates the threads Buffer: For the circular array Order: Stores what food from what till Tills: Creates an order Worker: Processes an order

Semaphore:

package fastfood;

public class Semaphore {

    private int count;

    public Semaphore(int n) {
        count = n;
    }

    public synchronized void down() {

        while (count == 0) {

            try {
                wait(); // Blocking call.
            } catch (InterruptedException exception) {
                exception.printStackTrace();
            }
        }
        count--;
    }

    public synchronized void up() {
        count++;
        notify();
    }
}

Buffer:

public class Buffer {
    private int size;
    private int inPtr = 0;
    private int outPtr = 0;
    private int counter = 0;
    private Order[] data; 
    private Semaphore sem = new Semaphore(1);

    public Buffer(int size) {
        this.size = size;
        this.data = new Order[size];
    }

    public Order remove(){
        // removes the revote for the officer 
        Order out;
        out = data[outPtr];
        System.out.println(" Food: " + out.getFoodId() + " TillId: " + 
            out.getTillId());
        outPtr = (outPtr+1)%size;
        counter--;
        return out;
    }
    public void insert(Order i){
        // inserts a new vote 
        data[inPtr] = i;
        inPtr = (inPtr+1)%size;
        counter++;
    }
    public boolean isEmpty(){
        // returns true if empty
        return counter==0;
    }
    public boolean isFull(){
        // returns true if full
        return counter==size;
    }
    public void acquire(){
        sem.down();
    }
    public void release(){
        sem.up();
    }

}

CHANGES:

Change 2:

Worker Class:

public void run() {
       while(FastFood.processedOrders < FastFood.totalOrders){
           try{
                buff.acquire();
                FastFood.semWorker.down();
                while(buff.isEmpty()){
                    try {
                        sleep(100);
                    } catch (InterruptedException ex) {
                        Logger.getLogger(Worker.class.getName()).log(Level.SEVERE, null, ex);
                    }
                }
                try{
                Order o = buff.remove();
                System.out.println(o.toString() + " FoodId: " + o.getFoodId() 
                        + " TillId: " + o.getTillId());
                FastFood.processedOrders++;
                }catch(Exception e){
                    System.out.println(e);
                }
           }finally{
               buff.release();
               FastFood.semTills.up();
           }

       }

Tills Class:

while (FastFood.startOrders < FastFood.totalOrders) {
            try {
                buff.acquire();
                FastFood.semTills.down();
                while (buff.isFull()) {
                    try {
                        sleep(100);
                    } catch (InterruptedException ex) {
                        Logger.getLogger(Tills.class.getName()).log(Level.SEVERE, null, ex);
                    }
                }
                try {
                    Order o = new Order(foodId, tillId);
                    System.out.println(o.toString());
                    buff.insert(o);
                    FastFood.startOrders++;
                } catch (Exception e) {
                    System.out.println(e);
                }
            } finally {
                buff.release();
                FastFood.semWorker.up(); 
            }

Order:

public class Order {
    private final int foodId;
    private final int tillId;
    static int count = 0;
    private int orderCount=0;

    public Order(int foodId, int tillId){
        this.foodId = foodId;
        this.tillId = tillId;
        this.orderCount = count++;
    }

    public int getFoodId() {
        return foodId;
    }

    public int getTillId() {
        return tillId;
    }

    public static int getCount() {
        return count;
    }

    public int getOrderCount() {
        return orderCount;
    }


    @Override
    public String toString() {
        return "FoodId: " +foodId+" TillId : "+tillId+" Order Count : "+ orderCount; 
    }

Answer 1

When Worker waits for order, it actually waits for one of several events:

1. buffer becomes non-empty.
2. All Tills finish their work, so no order will be generated ever.

It is better to combine all these events to single protection (Semaphore in your case). And move wait implementation into Buffer class:

public class Buffer {
    private int size;
    private int inPtr = 0;
    private int outPtr = 0;
    private int counter = 0;
    private Order[] data; 
    private Semaphore sem = new Semaphore(1);

    private bool isFinished; /* Whether no futher Orders will be added */
    public Buffer(int size) {
        this.size = size;
        this.data = new Order[size];
    }

    /* Put Order into buffer. If buffer is full, wait.*/
    public void put(Order i){
        sem.down();

        while(counter == size) // Buffer is full
        {
            // Some sort of busy wait
            sem.up();
            sleep(100);
            sem.down();
        }

        data[inPtr] = i;
        inPtr = (inPtr+1)%size;
        counter++;

        sem.up();
    }

    /* 
     *  Get order from the buffer and remove it.
     *  If buffer is empty and not finished, wait.
     *  Return null if buffer is empty and finished.
     */
    public Order get(){
        sem.down();

        while(counter == 0 && !isFinished)
        {
            // Some sort of busy wait
            sem.up();
            sleep(100);
            sem.down();
        }

        Order out;
        if(counter) // Otherwise `isFinished` is set and null should be returned.
        {
            out = data[outPtr];

            System.out.println(" Food: " + out.getFoodId() + " TillId: " + 
                out.getTillId());
            outPtr = (outPtr+1)%size;
            counter--;
        }

        sem.up();
        return out;
    }

    /* Mark buffer as finished. */
    void finish(void)
    {
        sem.down();
        isFinished = true;
        sem.up();
    }
}

Note, how busy waiting is performed: the semaphore is released, a thread sleeps some time and then the semaphore is acquired again.

Next, it is better to incorporate all logic about officer into separate class. It will be simple, but will consume semTills global semaphore:

class Officer
{
    private int totalOrders;
    private int startOrder;
    private Semaphore sem = new Semaphore(1);

    public Officer(int totalOrders) {
        this.totalOrders = totalOrders;
    }

    /* Return true if order is allowed, false otherwise. */
    public bool getOrder(void) {
        bool result;
        sem.down();
        if(startOrders != totalOrders) {
            startOrders++;
            result = true;
        }

        sem.up();
        return result;
    }
}

As you can see, checking startOrders and modifying it should come under single critical section.

Next, Tills should wait, while order, produced by it, is processed. Such one-shot waiting can be implemented using initially locked Semaphore:

public class Tills extends Thread {
    private final Buffer buff;
    private final int foodId;
    private final int tillId;
    private final Semaphore waiter = Semaphore(0); // Initially locked!

    public Tills(int tillId, int foodId, Buffer buff) {
        this.tillId = tillId;
        this.foodId = foodId;
        this.buff = buff;
    }

    @Override
    public void run(){
        while(FastFood.officer.getOrder()){
            Order v = new Order(foodId, tillId);
            System.out.println(v.toString());            

            Random n = new Random();
            int time = n.nextInt(100) + 1;
            sleep(time);

            buff.put(v);

            //Wait order to be processed
            waiter.down();
        }
    }
    /* Tell that order, added to the buffer, is processed. */
    public markOrderProcessed(void) {
        waiter.up();
    }
}

Note, that implementation becomes much more simpler. Also class publishes method markOrderProcessed for being called by the Worker.

Because worker gets only order from the buffer, this object (of type Order) should contain reference to the Tills, created it. Also, Order objects can now be created concurrently. So its static count field should be protected by Semaphore.

public class Order {
    ...
    private Tills till; // New member
    static private Semaphore sem = Semaphore(1); // Protect *count* field

    public Order(..., Tills till) { // New parameter to the constructor
        ...
        this.till = till;

        // `count` should be incremented under protection.
        sem.down();
        this.orderCount = count++;
        sem.up();
    }

    /* New method: mark order as processed. */
    public markProcessed(void)
    {
        till.markOrderProcessed();
    }
}

Now everything is read for implement Worker's run method. Note, that now this method doesn't use any sync directly, everything is done in low-level classes:

public void run(){
    Order order;
    while((order = buff.get()) != null) {
        System.out.println("Worker: " + workerId + " " + order);

        Random n = new Random();
        int time = n.nextInt(100) + 1;
        sleep(time);

        order.markProcessed(); // Mark order as processed, so tills can continue.
    }
}

And the main class. Note, that buffer is marked as finished only after all Tills are finished (joined):

public class FastFood {
    static Buffer buff = new Buffer(2);
    static Officer officer = new Officer(10);

    public static void main(String[] args) {
        // TODO code application logic here

        int numberOfWorkers = 2;
        int numberOfTills = 3;
        int numberOfFoodChoices = 4;
        Random rand = new Random();

        Tills[] tills = new Tills[numberOfTills];
        Worker[] workers = new Worker[numberOfWorkers];

        //int tillId, int foodId, Buffer buff
        for (int i = 0; i < tills.length; i++) {
            int foodId = rand.nextInt(numberOfFoodChoices) + 1;
            tills[i] = new Tills(i, foodId, buff);
            tills[i].start();
        }

        //int workerId, Buffer buff
        for (int i = 0; i < workers.length; i++) {
            workers[i] = new Worker(i, buff);
            workers[i].start();
        }

        for (Tills till : tills) {
            try {
                till.join();
            }catch (InterruptedException ex) {
                System.out.println(ex);
            }
        }

        /* 
         * Mark buffer as finished.
         * 
         * Workers, which found the buffer empty, may safetly stop now.
         */
        buff.finish();

        for (Worker worker : workers) {
            try {
                worker.join();
            }catch (InterruptedException ex) {
                System.out.println(ex);
            }
        }
    }
}

Answer 2

Is there any reason why you are releasing your locks right after your while loop? Your tills are releasing them right after you check in your while loop. This seems very confusing. You want your till threads to sleep after their order is done and only awoken when their order is complete? And you want your workers to only specifically work on a certain order? Shouldn't the workers be able to work on any order as long as there are orders to waiting in your buffer? Sorry, I can't post in comments since I don't have 50 rep.

Fast Food

import java.util.*;

public class FastFood {

/**
 * @param args the command line arguments
 */
static Buffer buff = new Buffer(2);
static Semaphore semWorker = new Semaphore(2);
static Semaphore semTills = new Semaphore(2);
static int totalOrders = 10;
static int startOrders = 0;
static int processedOrders = 0;

public static void main(String[] args) {
    // TODO code application logic here

    int numberOfWorkers = 2;
    int numberOfTills = 3;
    int numberOfFoodChoices =4;
    Random rand = new Random();

    Tills[] tills = new Tills[numberOfTills];
    Worker[] workers = new Worker[numberOfWorkers];

    //int tillId, int foodId, Buffer buff
    for (int i = 0; i < tills.length; i++) {
        int foodId = rand.nextInt(numberOfFoodChoices) + 1;
        tills[i] = new Tills(i, foodId, buff);
        tills[i].start();
    }

    //int workerId, Buffer buff
    for (int i = 0; i < workers.length; i++) {
        workers[i] = new Worker(i, buff);
        workers[i].start();
    }

    for (Tills till : tills) {
        try {
            till.join();
        }catch (InterruptedException ex) {
            System.out.println(ex);
        }
    }

    for (Worker worker : workers) {
        try {
            worker.join();
        }catch (InterruptedException ex) {
            System.out.println(ex);
        }
    }
}
}

Buffer

public class Buffer {
private int size;
private int inPtr = 0;
private int outPtr = 0;
private int counter = 0;
private Order[] data; 


public Buffer(int size) {
    this.size = size;
    this.data = new Order[size];
}

public synchronized String remove(){
    // removes the revote for the officer 
    Order out;
    out = data[outPtr];
    outPtr = (outPtr+1)%size;
    counter--;
    return " Food: " + out.getFoodId() + " ordered by TillId: " + out.getTillId();
}
public synchronized void insert(Order i){
    // inserts a new vote 
    data[inPtr] = i;
    inPtr = (inPtr+1)%size;
    counter++;
}
public synchronized boolean isEmpty(){
    // returns true if empty
    return counter==0;
}
public synchronized boolean isFull(){
    // returns true if full
    return counter==size;
}


}

Tills

public class Tills extends Thread {
private final Buffer buff;
private final int foodId;
private final int tillId;

public Tills(int tillId, int foodId, Buffer buff) {
    this.tillId = tillId;
    this.foodId = foodId;
    this.buff = buff;
}

public void run(){
    while(FastFood.startOrders < FastFood.totalOrders){
        FastFood.semTills.down();

        while(buff.isFull()){
            try {
                sleep(100);
            } catch (InterruptedException ex) {
                //Logger.getLogger(Tills.class.getName()).log(Level.SEVERE, null, ex);
            }
        }
        FastFood.startOrders++;
        Order v = new Order(foodId, tillId);
        buff.insert(v);
        System.out.println("Till number " + tillId + " created a new order " + foodId + " to be processed");
        FastFood.semWorker.up();

    }
}
}

Worker

public class Worker extends Thread{
private final int workerId;
private final Buffer buff;


public Worker(int workerId, Buffer buff) {
    this.workerId = workerId;
    this.buff = buff;
}

 public void run() {
    while (FastFood.totalOrders > FastFood.processedOrders) {
        FastFood.semWorker.down();
        while (buff.isEmpty()) {
            try {
                sleep(100);
            } catch (InterruptedException ex) {
                //Logger.getLogger(Worker.class.getName()).log(Level.SEVERE, null, ex);
            }
        }
        //FastFood.processedOrders++;
        System.out.println("Worker: " + workerId + " completed order number " + buff.remove() + " total orders processed so far: " + FastFood.processedOrders++);

        FastFood.semTills.up();
        try {
            Random n = new Random();
            int time = n.nextInt(100) + 1;
            sleep(time);
        } catch (InterruptedException ex) {
            System.out.println(ex);
        }
    }
 }
}

Not sure if this is like your order class

public class Order{
private int tillID;
private int foodID;
public Order(int food, int till){
    tillID = till;
    foodID = food;
}

int getFoodId(){
    return foodID;
}

int getTillId(){
    return  tillID;
}

}

Before you try it out do note that it's not 100% correct. I got rid of the semaphore in the buffer and just made the methods synchronized. If you changed the semaphore values to 1 in the fastfood it would not run completely because not all the threads are able to wake back up to join at the end to exit the program.

Also, using static variables for totalOrders and processessOrders as a way to control when your threads stop running seem concerning because I think that threads each have their own copy so it might also lead to race conditions. I might be wrong though. I'm not sure what you haven't looked at yet but I think this has some good information that might help