Unable to achive synchronization in multithreading

Question

I'm basically trying to implement a real life example of multiple people booking a single taxi online. Here in my code I have 3 classes- taxi,customer and server.
There has to be multiple customers(threads) and a single taxi. But I'm unable to do this. Everytime I create new customer, a new instance of taxi is created.
This is taxi class code-

    public class taxi  {
        boolean BOOKED=false;
         String id;
        void book(){
            try {
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
            BOOKED=true;
            System.out.println("Customer "+Thread.currentThread().getName()+" BOOKED taxi");
        }

        void release(){
            BOOKED=false;
            System.out.println("Customer "+Thread.currentThread().getName()+" RELEASED taxi");
        }

        void setId(String id){
            this.id=id;
        }

        String getId(){
            return id;
        }

    }

customer class code-

public class customer extends Thread {
     taxi t=new taxi();
        public  void run(){
            //System.out.println(t.hashCode());
            t.setId(Thread.currentThread().getName());
            System.out.println("Customer "+Thread.currentThread().getName()+" trying to BOOK taxi");
            t.book();
            System.out.println("Customer "+Thread.currentThread().getName()+" is currently USING taxi");

            try {


Thread.sleep(2000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("Customer "+Thread.currentThread().getName()+" RELEASING taxi");
        t.release();
        try {
            Thread.sleep(3000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("taxi used by customer "+Thread.currentThread().getName()+" set id to "+t.getId());
    }



}

server class code-

public class server {

public static void main(String args[]){

         customer A=new customer();
         customer B=new customer();
         customer C=new customer();
         customer D=new customer();
        Thread t=new Thread();
         A.setName("A");
         B.setName("B");
         C.setName("C");
         D.setName("D");

         A.start();
         B.start();
         C.start();
         D.start();




    }


    }

This is my output-

Customer B trying to BOOK taxi
Customer D trying to BOOK taxi
Customer A trying to BOOK taxi
Customer C trying to BOOK taxi
Customer B BOOKED taxi
Customer A BOOKED taxi
Customer A is currently USING taxi
Customer D BOOKED taxi
Customer D is currently USING taxi
Customer B is currently USING taxi
Customer C BOOKED taxi
Customer C is currently USING taxi
Customer C RELEASING taxi
Customer C RELEASED taxi
Customer D RELEASING taxi
Customer D RELEASED taxi
Customer A RELEASING taxi
Customer A RELEASED taxi
Customer B RELEASING taxi
Customer B RELEASED taxi
taxi used by customer D set id to D
taxi used by customer C set id to C
taxi used by customer A set id to A
taxi used by customer B set id to B

As you can see id of each taxi is different instead of having the same.
Kindly help.

What do you think `taxi t=new taxi();` does as a field within `customer`? — Sotirios Delimanolis, Dec 13 '15 at 05:33
it creates a instance of taxi in customer. I'm aware that it is the root cause of my problem but I can't figure out any other way of doing this. — Shivam Aggarwal, Dec 13 '15 at 05:34
Create a single `taxi` instance and share it among your `customer` instances. Have you learned about constructor arguments before? — Sotirios Delimanolis, Dec 13 '15 at 05:37
Yes I know about customer arguments. Can you please tell me the exact code of doing this. — Shivam Aggarwal, Dec 13 '15 at 05:39
Unfortunately, no. Use a constructor argument to set a `taxi` field in each of your `customer` instances. Use the same `taxi` instance as an argument to each `customer` you create. — Sotirios Delimanolis, Dec 13 '15 at 05:41
Also is there any other way to do this apart from passing the same instance of taxi class to all my customer constructors ? — Shivam Aggarwal, Dec 13 '15 at 05:48
I've fixed the above code.Now how do I use methods like wait() and notify() in this example. — Shivam Aggarwal, Dec 13 '15 at 06:24

score 1 · Answer 1 · answered Dec 13 '15 at 06:10

1

I think, Taxi class need not be a thread. by making taxi class as shared resource(single ton with synchronized method) among all the other customer threads we can achieve the expected synchronization in multi threading.

answered Dec 13 '15 at 06:10

Srinu Yarru

61
1

Can you please tell me how do I use methods like wait() and notify() here – Shivam Aggarwal Dec 13 '15 at 06:25

akki · Accepted Answer · 2015-12-13T11:38:46.077

Some important points regarding your code:

You should create only a single instance of Taxi class and remove the taxi instance variable from Customer class and instantiate Taxi in server class.
change your Customer class to work on a shared taxi. You can create a parameterized constructor in Customer class to initialize the shared taxi.
setId should be called inside the method book so that Id of the taxi can only be changed by the thread that wants to book taxi.

You can use wait/notify mechanism this way in Taxi class to achieve synchronization:

public class Taxi {
    Boolean BOOKED = false;
    String id;

    void book() throws InterruptedException {
        synchronized (this) {
            while (BOOKED) {
                this.wait();
            }
            try {
                setId(Thread.currentThread().getName());
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            BOOKED = true;
            System.out.println("Customer " + Thread.currentThread().getName() + " BOOKED taxi");
            try {
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
        }
    }

    void release() throws InterruptedException {
        synchronized (this) {
            BOOKED = false;
            System.out.println("Customer " + Thread.currentThread().getName() + " RELEASED taxi");
            this.notifyAll();
        }
    }

    void setId(String id) throws InterruptedException {
        System.out.println("SETTING ID TO CUSTOMER " + Thread.currentThread().getName());
        this.id = id;
    }

    String getId() {
        return id;
    }
}

Customer:

   public class Customer extends Thread {
    Taxi taxi;

    public Customer(Taxi taxi){
        this.taxi = taxi;
    }

    public  void run(){
        //System.out.println(t.hashCode());

        System.out.println("Customer "+Thread.currentThread().getName()+" trying to BOOK taxi");
        try {
            taxi.book();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("Customer "+Thread.currentThread().getName()+" is currently USING taxi");

        try {
            Thread.sleep(2000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("Customer "+Thread.currentThread().getName()+" RELEASING taxi");
        try {
            taxi.release();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        try {
            Thread.sleep(3000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}

I guess you misunderstood the problem. The problem here is that I end up having multiple instances of taxi class and since, taxi class acts as my data I cannot have multiple instances of it. — Shivam Aggarwal, Dec 13 '15 at 09:44
You should create only a single instance of Taxi class and remove the taxi instance variable from Customer class and instantiate Taxi in server class — akki, Dec 13 '15 at 09:46
Okay. Then how do I use that instance further, so that all threads work on that same instance — Shivam Aggarwal, Dec 13 '15 at 09:49

score 1 · Answer 3 · answered Dec 13 '15 at 10:16

Why you create every time a new taxi in customer? The problem is: taxi t=new taxi();. Just pass the taxi by constructor or make it global or something like that. In that way every customer will use the same taxi. Furthermore Threads are dangerous, so you should be very careful with them. You don't have any synchronization. I suggest you don't understand how to properly use threads. I suggest you to read about synchronize, volatile, wait, notify, notifyAll and to experiment and try some basic examples. I thing the first two was described well in Head First Java. Offtopic : you should follow conventions when naming your variables and methods.

Unable to achive synchronization in multithreading

3 Answers3