Some threads gets stuck at semaphore.aquire() (threads/semaphore/countdownlatch)

Question

I've created a small movie rental simulation program. Here's how it works: - The main thread lets the user input customers names

• Every customer typed in launches a new thread (the Customer Runnable)
• When 5 customers have been created, the rental service starts (countdownlatch of 5 being waited for)
• When the customers run(), they will first try to aquire() a permit from the semaphore (that has 5 permits available)
• If they get a permit, they will wait 1-10 seconds, then rent a car, then wait 1-3 seconds, and deliever the car
• When the car is delievered, they will start the loop iteration all over and try to get a new permit

So this seems to work totally fine; it works for the first 5 customers being added. The customers being added after the 5th seems to get stuck wait at the semaphore.aquire(), and i cannot understand why, so i'm asking here. All help would be VERY highly appreciated :)

App.java:

import java.lang.System;import java.util.Scanner;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.Semaphore;

public class App {

    public static CountDownLatch latch = new CountDownLatch(5);
    public static Executor executor = Executors.newCachedThreadPool();
    public static Store store = new Store();
    public static Semaphore semaphore = new Semaphore(Store.getMovies().size());

    Scanner in;

    public App() {
        in = new Scanner(System.in);

        while (true) {
            executor.execute(new Customer(in.nextLine()));
        }
    }

    public static void main(String[] args) {
        new App();
    }

    public CountDownLatch getLatch() {
        return latch;
    }

    public Executor getExecutor() {
        return executor;
    }

    public Semaphore getSemaphore() {
        return semaphore;
    }

}

Customer.java:

public class Customer implements Runnable {

    String name;

    public Customer(String name) {
        this.name = name;
    }

    public void run() {
        try {
            App.latch.countDown();
            App.latch.await();
        } catch (InterruptedException e) {
            System.out.println(e.getMessage());
            e.printStackTrace();
        }

        // Loop until ended
        while (true) {
            try {
                if (App.semaphore.availablePermits() == 0)
                    System.out.println("No available movies");

                // Acquire permit
                App.semaphore.acquire();

                // Sleep from 1-10 seconds before renting a Car
                int rand = 1 + (int) (java.lang.Math.random() * 10);
                Thread.sleep(rand * 1000);
                App.store.rent(this);

                // Sleep from 1-3 seconds before delivering the Car
                rand = 1 + (int) (Math.random() * 3);
                Thread.sleep(rand * 1000);
                App.store.deliver(this);

            } catch (InterruptedException e) {
                e.printStackTrace();
            } finally {
                App.semaphore.release();
            }
        }
    }

    public String getName() {
        return name;
    }
}

Store.java:

import java.lang.String;import java.lang.System;import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;


public class Store {

    private static List<Movie> movies;

    private static Lock lock = new ReentrantLock();

    public Store() {
        movies = new ArrayList<Movie>();
        movies.add(new Movie("Godfather"));
        movies.add(new Movie("LOTR"));
        movies.add(new Movie("Schindlers list"));
        movies.add(new Movie("Pulp fiction"));
        movies.add(new Movie("Fight club"));
    }

    public void rent(Customer c) {
        lock.lock();
        for (Movie movie : movies) {
            if (movie.getRentedBy() == null) {
                movie.setRentedBy(c);
                String str = c.getName() + " rented " + movie.getName();
                System.out.printf("%-30s", str);
                printStatus();
                break;
            }
        }
        lock.unlock();
    }

    // Deliver the Car
    public void deliver(Customer c) {
        lock.lock();
        for (Movie movie : movies) {
            if (movie.getRentedBy() != null && movie.getRentedBy().equals(c)) {
                movie.setRentedBy(null);
                String str = c.getName() + " delivered " + movie.getName();
                System.out.printf("%-30s", str);
                printStatus();
                break;
            }
        }
        lock.unlock();
    }

    public void printStatus() {
        String str;
        for (Movie m : movies) {
            System.out.print(m.getName() + " - ");
            if (m.getRentedBy() == null) {
                str = "available";
            } else {
                str = "rented by " + m.getRentedBy().getName();
            }
            System.out.printf("%-15s", str);
        }
        System.out.println();
    }

    public static List<Movie> getMovies() {
        return movies;
    }
}

Movie.java:

public class Movie {

    private String name;
    private Customer rentedBy;

    public Movie(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }

    public Customer getRentedBy() {
        return rentedBy;
    }

    public void setRentedBy(Customer customer) {
        this.rentedBy = customer;
    }

}

Answer 1

Try adding true as the second parameter on the Semphore constructor call.

By default, there is no attempt at fairness, which you need to get all renters taking turns. Generally, a renter that has just returned a movie will get to the acquire call faster than one that was waiting for the semaphore. With the added true argument "this semaphore will guarantee first-in first-out granting of permits under contention" Semaphore

Answer 2

The problem with your code is that your Customer Thread runs infinite loop and immediately tries to acquire the semaphore after release (Another way to do is that Customer Thread should do its business and terminate). The 6th Thread is actually waiting for its turn but likelihood to acquire permit is less since first 5 threads are active. To check this you can put Thread to timed sleep after releasing semaphore permit.

Also, the latch is being used in a wrong way. Caller should be calling await once that waits for for all 5 threads to call countdown