Multi-Thread Producer/Consumer Synchronization Issues

Question

I am having issues with two threads that dont seem to synchronize properly. I basically have an boolean value names "occupied". It is set to false when no thread is started. but when one starts, the thread sets occupied is true I have a class that has the thread (run) and they call the functions below.

This is a mock bank example that takes in an amount (initial balance) and then performs withdrawals and deposits randomly. My professor mention something about signaling form the withdraw thread to the deposit thread? How does that work? The withdraw thread, it should run until the balance is two low and wait for a deposit thread. How should i do that?

   package bank;

import java.util.Random;
import bank.Bank;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.Condition;

/**
 *
 * @author KJ4CC
 */
public class Action {

    private Lock accessLock = new ReentrantLock();
    private Condition cond = accessLock.newCondition();
    //private Condition withdraw  = accessLock.newCondition();


    Random rand = new Random();
    Object lock = new Object();
    Bank getBalance = new Bank();

    public void widthdrawl(int threadNum) throws InterruptedException {
        int amount = rand.nextInt(50);
            accessLock.lock();

            if (getBalance.getbalance() > amount) {

                getBalance.setBalance(getBalance.getbalance() - amount);

                System.out.println("\t\t\tThread " + threadNum + " withdrawls " + amount + "\t Balance is " + getBalance.getbalance());

            } else {

                System.out.println("\t\t\tThread " + threadNum + " Failed to withdrawl " + amount + "\t Balance is " + getBalance.getbalance());
                cond.await();

            }


            accessLock.unlock();
            Thread.sleep(rand.nextInt(5));

    }

    public void deposit(int threadNum) throws InterruptedException {
        int amount = rand.nextInt(200);
        accessLock.lock();


            getBalance.setBalance(getBalance.getbalance() + amount);
            System.out.println("Thread " + threadNum + " Deposits " + amount + "\t\t\t\t Balance is " + getBalance.getbalance());
            Thread.sleep(rand.nextInt(100));

            cond.signal();
            accessLock.unlock();


    }
}

Answer 1

Your Lock and Condition usage is wrong. You're calling lock() without calling unlock() anywhere, and you're calling signal() without anyone having called await().

See the documentation for an example very much related to your problem.

Answer 2

First, you have to mark you occupid variable volatile. Without this keyword changing the variable value in one thread won't be visible in another.

Second, you trying to implement external synchronization policy for bank entity. Such idea basically isn't good: if someone uses the same bank without properly synchronization it'll break internal bank state. It's better to implement internal synchronization policy and allow bank to guard its state by itself.

E.g. the API of Bank class may be like this

class Bank {
   double synchronize getBalance() { ... }
   void synchronize deposit(double amount) { ... }
   void synchronize widthdrawl(double amount) throw BankException { ... }
}

With this design internal Bank state always be consistent and any Bank users will be wait finishing current bank operation automatically.