Java MultiThreading .wait() .notify()

Question

I have been working on a long project trying to understand the basics of multi-threading. The application is supposed to simulate a Shop and a Customer which share a Box and a PayPal account.

Main.Java

package exe5;

    public class Main {

        static Thread mainThread = Thread.currentThread();

        public static void main(String[] args) throws InterruptedException{
            Box box = new Box();
            PayPalAcc pp1 = new PayPalAcc();
            mainThread.setName("mainThread");

            Thread DaniCustomerThread = new Thread(new Customer(box, pp1, "Daniel Netzer"), Customer.getName());
            Thread POShopThread = new Thread (new Shop(box, pp1, "Post Office"), Shop.getName());

            DaniCustomerThread.start();
            POShopThread.start();

            Thread.sleep(5);

            // Closing all loops and adding signature to log of safe closure.
            DaniCustomerThread.interrupt();
            POShopThread.interrupt();

            Thread.sleep(100); // Allowing main thread a bit more to actually get the signature for safe closure for both threads.
        }

    }

Customer.java

package exe5;

public class Customer implements Runnable{

    private static Box box;
    private static PayPalAcc paypal;
    private static String name;
    private static Object customerLock = new Object();

    public Customer(Box box,PayPalAcc paypal, String name){
        Customer.box = box;
        Customer.name = name;
        Customer.setPaypal(paypal);
    }

    public static Object getCustomerLockMonitor(){
        return Customer.customerLock;
    }

    public static String getName() {
        return name;
    }

    // Unused Getters/Setters
    /*private synchronized void setName(String name) {
        Customer.name = name;
    }

    private static Box getBox() {
        return box;
    }

    private synchronized void setBox(Box box) {
        Customer.box = box;
    }*/

    private static PayPalAcc getPaypal() {
        return paypal;
    }

    private static synchronized void setPaypal(PayPalAcc paypal) {
        Customer.paypal = paypal;
    }

    private synchronized void  printsCustomer(int caseNum){
        switch(caseNum){
        case 1: System.out.println(Customer.getName() +" package have been withdrawed from his box at " +Shop.getName());
                break;
        case 2: System.out.println(Customer.getName() +" deposited money into shared PayPal account.");
                break;
        case 3: System.out.println(Customer.getName() +" box is empty, waiting for a new package to arrive.");
                break;
        case 4: System.out.println("closing safely " +Thread.currentThread().getName() +" thread.");
                break;
        }
    }

    private synchronized void  withdrawBox(){
        Customer.box.setBoxStatus(false);
        printsCustomer(1);
        synchronized(Shop.getShopBoxMonitor()){
            Shop.getShopBoxMonitor().notify();}
    }

    private synchronized void depositInPayPal(int amountToDeposit){
        Customer.getPaypal().setPayPalAccStatus(Customer.getPaypal().getPayPalAccStatus() + amountToDeposit);
        printsCustomer(2);
        synchronized (Shop.getShopPPMonitor()){
            Shop.getShopPPMonitor().notify();}
    }

    @Override
    public void run() {
        while(Main.mainThread.isAlive()){
            while(!box.isBoxStatus()){
                synchronized(Customer.getCustomerLockMonitor()){
                    try {
                        printsCustomer(3);
                        depositInPayPal(100);
                        Customer.getCustomerLockMonitor().wait();
                    } catch (InterruptedException e) {
                        printsCustomer(4);
                        Thread.currentThread().interrupt();
                        break;
                    }
                }
            }
            if (Thread.currentThread().isInterrupted()) { break; }
            withdrawBox();
        }
    }
}

Shop.java

package exe5;

public class Shop implements Runnable{

    private static Box box;
    private static PayPalAcc paypal;
    private static String name;
    private static Object shopBox = new Object();
    private static Object shopPP = new Object();

    public Shop(Box box,PayPalAcc paypal, String name){
        Shop.box = box;
        Shop.name = name;
        Shop.setPaypal(paypal);
    }

    public static Object getShopBoxMonitor(){
        return Shop.shopBox;
    }

    public static Object getShopPPMonitor() {
        return Shop.shopPP;
    }

    public static void setShopPP(Object shopPP) {
        Shop.shopPP = shopPP;
    }

    public static String getName() {
        return name;
    }

    // Unused Getters/Setters
    /*private synchronized void setName(String name) {
        Shop.name = name;
    }

    private static Box getBox() {
        return box;
    }

    private synchronized void setBox(Box box) {
        Shop.box = box;
    }*/

    private static PayPalAcc getPaypal() {
        return paypal;
    }

    private static synchronized void setPaypal(PayPalAcc paypal) {
        Shop.paypal = paypal;
    }

    private synchronized void  depositBox(){
        Shop.box.setBoxStatus(true);
        printsShop(1);
        synchronized(Customer.getCustomerLockMonitor()){
            Customer.getCustomerLockMonitor().notify();}
    }

    private synchronized void withdrawFromPayPal(int amountToWithdraw){
        Shop.getPaypal().setPayPalAccStatus(Shop.getPaypal().getPayPalAccStatus() - amountToWithdraw);
        printsShop(2);
    }

    private synchronized void  printsShop(int caseNum){
        switch(caseNum){
        case 1: System.out.println(Shop.getName() +" deposited new package in " +Customer.getName() +" box.");
                break;
        case 2: System.out.println(Shop.getName() +" withdrawed money from shared PayPal account.");
                break;
        case 3: System.out.println(Shop.getName() +" box is full, waiting for customer withdrawal.");
                break;
        case 4: System.out.println(Customer.getName() +" did not deposited money into PayPal account yet.");
                break;
        case 5: System.out.println("closing safely " +Thread.currentThread().getName() +" thread.");
                break;
        }
    }

    @Override
    public void run() {
        while(Main.mainThread.isAlive()){
            while(Shop.getPaypal().getPayPalAccStatus() < 100){
                synchronized(Shop.getShopPPMonitor()){
                    try {
                        printsShop(4);
                        Shop.getShopPPMonitor().wait();
                    } catch (InterruptedException e) {
                        printsShop(5);
                        Thread.currentThread().interrupt();
                        break;
                    }
                }
                withdrawFromPayPal(100);
                while(box.isBoxStatus()){
                    synchronized(Shop.getShopBoxMonitor()){
                        try {
                            printsShop(3);
                            Shop.getShopBoxMonitor().wait();
                        } catch (InterruptedException e) {
                            printsShop(5);
                            Thread.currentThread().interrupt();
                            break;
                        }
                    }
                }
                depositBox();
            }
            if (Thread.currentThread().isInterrupted()) { break; }
        }
    }
}

Box.java

package exe5;

public class Box {

    private static boolean boxStatus;

    public Box(){
        Box.boxStatus = false;
    }

    public boolean isBoxStatus() {
        return boxStatus;
    }

    public void setBoxStatus(boolean boxStatus) {
        Box.boxStatus = boxStatus;
    }

}

PayPalAcc.java

package exe5;

public class PayPalAcc {

    private static int payPalAccStatus;

    public PayPalAcc(){
        this.setPayPalAccStatus(0);
    }

    public int getPayPalAccStatus() {
        return payPalAccStatus;
    }

    public void setPayPalAccStatus(int payPalAccStatus) {
        PayPalAcc.payPalAccStatus = payPalAccStatus;
    }

}

Try to write a title that summarizes what your code does. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. — BCdotWEB, Dec 29 '15 at 15:26
Welcome to Code Review! I have rolled back the last edit. Please see what you may and may not do after receiving answers. — Quill, Dec 30 '15 at 14:31

VoiceOfUnreason · Accepted Answer · 2015-12-29 23:08:53Z

        Thread DaniCustomerThread = new Thread(new Customer(box, pp1, "Daniel Netzer"), Customer.getName());
        Thread POShopThread = new Thread (new Shop(box, pp1, "Post Office"), Shop.getName());

Working with threads directly is usually a bad idea. Much better to defer the thread management to an ExecutorService, and let that service control the thread management for you. The startup code would normally look like

// There are several kinds of Executor available, choose what is suitable.
ExecutorService executor = Executors.newCachedThreadPool();
executor.submit(new Customer(box, pp1, "Daniel Netzer"));
executor.submit(new Shop(box, pp1, "Post Office"));

This will start your tasks running in the background. When you want to cancel the execution of the tasks, you shutdown the Executor

 // Not executor.shutdown(), which has a different purpose
executor.shutdownNow();

shutdownNow will interrupt all of the running threads.

In doing this, we have lost for the moment the ability to control the thread name. There are ways of configuring the ThreadFactory used by the executor, but they don't fit your use case very well. Instead, you should borrow this good idea:

mainThread.setName("mainThread");

But instead of applying it to the main thread (where, to be honest, I'm not sure there is a lot of added value), you add it to the logic of the run() methods

public class Customer implements Runnable{
    @Override
    public void run() {
        Thread.currentThread().setName(this.getName());
        // ...
    }
}

Your loops within the run methods are a bit weird...

public void run() {
    while(Main.mainThread.isAlive()){
        // ...
    }
}

I'm not certain that idiom is wrong; I'll only note that when I've been looking for best practices in controlling loops, I've never seen a call to Thread.isAlive. More common spellings

// run "forever", with internal breaks to shut things down
while (true) {
    ...
}

// Interruption equal Cancellation
while (! Thread.currentThread().interrupted()) {
    ...
}

// volatile state to track changes signalled by other threads
while (this.running) {
    ...
}

// more flexible, perhaps if there's a state machine somewhere
while (this.isRunning()) {
    ...
}

More generally:

I have been working on a long project trying to understand the basics of multi-threading. The application is supposed to simulate a Shop and a Customer which share a Box and a PayPal account.

I don't think you have this straight yet. The customer and the shop are sharing these two resources. More specifically, they are trying to modify these two resources. So the locks you care about are the locks on the shared resources, not the locks on the resources doing the sharing.

The usual idiom looks something like this

// block until we can get exclusive control of the lock
synchronized (account) {

    // if we can't currently make progress... 
    while(insufficientFunds(account)) {

        // then release the lock, and block until something changes.
        account.wait ();
    }

    // Aha, we have control of the lock AND we know that there are sufficient
    // funds available

    debit(account);

    // we've made a change to the account balance, therefore we should
    // wake up the other threads that are blocking on it, in case they
    // care
    account.notifyAll();
} // release the lock

Additional comments

Customer and Shop probably aren't Runnables; but entities that run, or participate, in tasks. If you separate the task from the entity, things will probably become a lot more clear.

public class Customer {
    public void scheduleOrders(ExecutorService executor) {
        Runnable task = new OrderBoxesTask(this.box, this.paypal);
        executor.submit(task);
    }
}

public OrderBoxesTask implements Runnable {
    public void run() {
        //...
    }
}

Alternatively, maybe instead of Customer and Shop, you just have OrderingProcess and FulfillmentProcess; "naming things" is one of the two hard problems.

In your example, most (all? I got too scarred to look closely) of your classes are manipulating static variables instead of instance variables. In this case, you did more typing to make it harder to understand what was going on. If you employ the discipline of encapsulating the data, the compiler will help show you places where you don't understand what you need to do.

Consider the following code

Customer alice = new Customer(new Box(), new PayPalAcc(), "Alice");
Customer bob = new Customer(new Box(), new PayPalAcc(), "Bob");
System.out.println(alice.getName());

That's not going to do what people expect. The problem here is that you specified that name (and everything else) are static variables, which means that Bob's name wrote over Alice's.

When you are writing a class, you should assume that all state/properties are going to belong to an instance, until you know otherwise. So the variable declarations should not include the "static" keyword.

public class Customer implements Runnable{

    private Box box;
    private PayPalAcc paypal;
    private String name;

Also: spell your words PayPalAcc should be PayPalAccount, shopPP should be shopPayPal or shopPayPalAccountLock....

thanks alot, I'm about to completely apply everything you said to my project especially the idea with the account.notify/wait / box. notify/wait., i just didnt realy understood that last few things, what do you mean "probabaly aren't Runnables" ? and do you mind giving a good example of instace variables vs. static variables? cause I clearly dont realy understand the basic idea behind it. — Daniel Netzer, Dec 29 '15 at 22:00
completely understood everything and on my way to apply everything in my new code. thank you very much mate. — Daniel Netzer, Dec 29 '15 at 23:17
posted revised code according to VoiceOfUnreason tips. codereview.stackexchange.com/questions/115429/… — Daniel Netzer, Dec 30 '15 at 16:12

asked	5 months ago
viewed	79 times
active	5 months ago

current community

your communities

more stack exchange communities

Java MultiThreading .wait() .notify()

1 Answer 1

Your Answer

Not the answer you're looking for? Browse other questions tagged java beginner multithreading or ask your own question.

Linked

Hot Network Questions

current community

your communities

more stack exchange communities

Java MultiThreading .wait() .notify()

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged java beginner multithreading or ask your own question.

Linked

Related

Hot Network Questions