Link to home
Start Free TrialLog in
Avatar of Simon Leung
Simon Leung

asked on

Thread Synchronization with equal sharing

In the code below, four threads are supposed to take turns to buy the ticket. However, it seems that only 2 threads buys more tickets than other.

Any idea how to communicate among the threads such that each one take turn to buy ?

Thx

class Buyer3 extends Thread {
   TicketPool pool;// declare total ticket and initialized it to 100 
     boolean neverBrought= true;
   
   Buyer3(TicketPool pool)
   {
      this.pool = pool;
   }
   
   public void run() //override run() here 
   {      
      //loop if availableTicket >0    
      
      while(true){
         
         synchronized(pool) {
            if (neverBrought == false) {
               try {
                  //wait for execution
                  pool.wait();
                                    
               }catch(InterruptedException e){
                  e.printStackTrace();
               }
            }
            
            
         
           if (pool.getTicket()>0) {
               
              // place order: print out "Thread xx reserved ticket --- #n"
             System.out.println(Thread.currentThread().getName()+" reserved ticket # "+ pool.getTicket());
         
              //delay 10ms: sleep(10)
            try{
                 Thread.currentThread().sleep(10);
               }
                catch (InterruptedException itro){
                 System.out.println("sleep is interrupted");
               }
         
                //payment finished: decrease availableTicket by 1 
                pool.setTicket(pool.getTicket()-1);
         
                //print the leftover tickets
                System.out.println(Thread.currentThread().getName()+" bought ticket # " + (pool.getTicket()+1) + ", ticket left: "+pool.getTicket());
               
               neverBrought=false;
               
               //notify execution
               pool.notify();
         
               
            }else {
               System.exit(0); 
            }

         }

         
      
         
      }// go back to buy another once (while loop)   

   }// end of override run()

}

class TicketPool {
   int availableTicket;
   
   TicketPool(int totalTicket)
   {
      this.availableTicket = totalTicket;
   }
   
   void setTicket(int newNumber)
   {
      availableTicket = newNumber;
   }
   
   int getTicket(){
      return availableTicket;
   }
      
}

public class TicketDemo3{
   
   public static void main (String[] args){
      // start 4 threads from a object of class Ticket that implements the "Runnable" interface
      TicketPool tickets = new TicketPool(10);
      
         
      // start 4 threads from the same object
      Buyer3 t1 = new Buyer3(tickets);
      Buyer3 t2 = new Buyer3(tickets);
      Buyer3 t3 = new Buyer3(tickets);
      Buyer3 t4 = new Buyer3(tickets);
      
      t1.setName("John");
      t2.setName("Mike");
      t3.setName("Lily");
      t4.setName("Mary");
      
      
      t1.start();
      t2.start();
      t3.start();
      t4.start();
   }
}

Open in new window


Avatar of ste5an
ste5an
Flag of Germany image

Maybe you explain the context and your assumptions.

Any idea how to communicate among the threads such that each one take turn to buy
In the given context so far, using threads makes then no longer sense as this is equal to sequential execution.
And in general only the pool can decide this, if it knows the buyers. Or you use a separate communication channel object, where the buyers talk to each other and decide who can buy.
You accepted my solution to this very question in an earlier question you posted. What has changed since then ? (I haven't examined your present code because of that )
Avatar of Simon Leung
Simon Leung

ASKER

My previous coding is based on this example. I find a new problem is that not all the threads are assessed the synchronzied code section. If 3 threads (clients) try to access the synchronized code, only two threads can enter alternatively. The third thread can enter this synchronized code only when either one of completely quit the apps.

The short  code that I follow also have this problem. Any idea ?

Thx again
Ahh, good point.

Yeah, you need to take Helgi's and DPearsons advice in the other thread serious. The method doing the decrease must be synchronized.

And for your question:
Using threads means executing code in parallel. But there is no guarantee that all threads get the same amount of CPU time, preemptive scheduling tries to do this. but this means on the higher levels not necessarily that the high level payloads are equally distributed.

Your code can be something like:

class Buyer3 extends Thread {
    TicketPool pool;
    boolean neverBrought= true;

    Buyer3(TicketPool pool)
    {
        this.pool = pool;
    }

    public void run()
    {
        while(true) {
            if (neverBrought == false) {
                pool.wait();
            }

            int ticketsAvailable = pool.getTicket();
            if (ticketsAvailable >0) {
                System.out.println(Thread.currentThread().getName()+" available tickets # "+ ticketsAvailable);
                Thread.currentThread().sleep(10);
                ticketsAvailable = pool.buyTicket();
                if (ticketsAvailable != -1) {
                    System.out.println(Thread.currentThread().getName()+" bought ticket. tickets left before buying: " + ticketsAvailable);
                    neverBrought=false;
                } else {
                    System.out.println(Thread.currentThread().getName()+" could not buy ticket.");
                }
                pool.notify();
            } else {
                System.exit(0);
            }
        }
    }
}

public class TicketPool {
    private int availableTicket;

    public TicketPool(int totalTicket)
    {
        this.availableTicket = totalTicket;
    }

    public int getTicket() {
        return this.availableTicket;
    }

    public int buyTicket(){
        int result = -1; // using magic.
        synchronized (this) {
            if (this.availableTicket > 0) {
                result = this.availableTicket--;
            }
        }

        return result;
    }
}

Open in new window

"you need to take Helgi's and DPearsons advice in the other thread serious ".
Is there any good reference that I can learn more to solve the problem ?

Thx again.
No, not that I'm aware of. Concurrency in parallelism is a pretty complex field in Information Science and the actual language. But your problem here is more simple:

Calls from different thread to the same class can interleave. Thus in your sample without synchronization

 if (pool.getTicket()>0) {              
              // place order: print out "Thread xx reserved ticket --- #n"
             System.out.println(Thread.currentThread().getName()+" reserved ticket # "+ pool.getTicket());

Open in new window

a second thread can call and execute  setTicket() between line 1 and 3.  But you should keep the synchronized code blocks as small as possible, Thus my proposition to do it in the decrease method.

See also https://docs.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html
Just replace your present Buyer3 with this


class Buyer3 extends Thread {

   TicketPool pool;// declare total ticket and initialized it to 100 
      
     
     static Object delta;
     
   
   Buyer3(TicketPool pool)
   {
   
        delta = new Object();
        this.pool = pool;
   }
   
   public void run() //override run() here 
   {      
      //loop if availableTicket >0    
      
      while(true){
      
        synchronized(delta) { 
               
         
           if (pool.getTicket()>0) {
               
              // place order: print out "Thread xx reserved ticket --- #n"
             System.out.println(Thread.currentThread().getName()+" reserved ticket # "+ pool.getTicket());
         
              //delay 10ms: sleep(10)
            try{
                 Thread.currentThread().sleep(10);
               }
                catch (InterruptedException itro){
                 System.out.println("sleep is interrupted");
               }
         
                //payment finished: decrease availableTicket by 1 
                pool.setTicket(pool.getTicket()-1);
         
                //print the leftover tickets
                System.out.println(Thread.currentThread().getName()+" bought ticket # " + (pool.getTicket()+1) + ", ticket left: "+pool.getTicket());
               
                              
                try{
                    delta.notifyAll();
                    delta.wait();
                }catch(Exception ex){}
                
                             
            }else {
               System.exit(0); 
            }
            
         }
         
      }// go back to buy another once (while loop)   

   }// end of override run()

}

Open in new window



Try that.
Result : Still no the expected result, it is supposed John, Mike, Mary, Lily, then repeat again...
John reserved ticket # 10
John bought ticket # 10, ticket left: 9
Mike reserved ticket # 9
Mike bought ticket # 9, ticket left: 8
Mary reserved ticket # 8
Mary bought ticket # 8, ticket left: 7
Lily reserved ticket # 7
Lily bought ticket # 7, ticket left: 6
Mary reserved ticket # 6
Mary bought ticket # 6, ticket left: 5
Mike reserved ticket # 5
Mike bought ticket # 5, ticket left: 4
John reserved ticket # 4
John bought ticket # 4, ticket left: 3
Mary reserved ticket # 3
Mary bought ticket # 3, ticket left: 2
Lily reserved ticket # 2
Lily bought ticket # 2, ticket left: 1
John reserved ticket # 1
John bought ticket # 1, ticket left: 0
Another Test :
John available tickets # 10
Mary available tickets # 10
Mike available tickets # 10
Lily available tickets # 10
John bought ticket. tickets left before buying: 10
Mary bought ticket. tickets left before buying: 7Exception in thread "John"
Exception in thread "Mary" Lily bought ticket. tickets left before buying: 9
Exception in thread "Lily" Mike bought ticket. tickets left before buying: 8
Exception in thread "Mike" java.lang.IllegalMonitorStateException
        at java.lang.Object.notify(Native Method)
        at Buyer3.run(TicketDemo3.java:42)
java.lang.IllegalMonitorStateException
        at java.lang.Object.notify(Native Method)
        at Buyer3.run(TicketDemo3.java:42)
java.lang.IllegalMonitorStateException
        at java.lang.Object.notify(Native Method)
        at Buyer3.run(TicketDemo3.java:42)
java.lang.IllegalMonitorStateException
        at java.lang.Object.notify(Native Method)
        at Buyer3.run(TicketDemo3.java:42)

Open in new window


class Buyer3 extends Thread {
    TicketPool pool;
    boolean neverBrought= true;

    Buyer3(TicketPool pool)
    {
        this.pool = pool;
    }

    public void run()
    {
        while(true) {
            if (neverBrought == false) {
                pool.wait();
            }

            int ticketsAvailable = pool.getTicket();
            if (ticketsAvailable >0) {
                System.out.println(Thread.currentThread().getName()+" available tickets # "+ ticketsAvailable);
                Thread.currentThread().sleep(10);
                ticketsAvailable = pool.buyTicket();
                if (ticketsAvailable != -1) {
                    System.out.println(Thread.currentThread().getName()+" bought ticket. tickets left before buying: " + ticketsAvailable);
                    neverBrought=false;
                } else {
                    System.out.println(Thread.currentThread().getName()+" could not buy ticket.");
                }
                pool.notify();
            } else {
                System.exit(0);
            }
        }
    }
}

public class TicketPool {
    private int availableTicket;

    public TicketPool(int totalTicket)
    {
        this.availableTicket = totalTicket;
    }

    public int getTicket() {
        return this.availableTicket;
    }

    public int buyTicket(){
        int result = -1; // using magic.
        synchronized (this) {
            if (this.availableTicket > 0) {
                result = this.availableTicket--;
            }
        }

        return result;
    }
}

Open in new window



Simon you're trying to use "wait" and "notify" to control the synchronization and you're subclassing the Thread class.
That would be the correct approach if you were trying to write this code in 1990.

In 2020, we don't use these primitives any more for concurrent programming.
Now there is an entire package java.util.concurrent:
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/util/concurrent/package-use.html#java.util.concurrent
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/package-summary.html
filled with tools to help.

I already suggested using one of these (the AtomicXYZ classes) in your earlier question.

There are lots of examples for how to use these here:
https://docs.oracle.com/javase/tutorial/essential/concurrency/

Please give these a try.  If you continue to write code that subclasses Thread and tries to use wait() and notify() directly, you are learning how to work with stone age tools in the electronic age.  Even if your code eventually works, you are developing the wrong skills.

(I write concurrent Java code all day every day and I've not used wait(), notify() or Thread directly in more than a decade).

Doug

Doug is right of course, but if you are trying to do this for other reasons (understanding legacy code, or insight into prior art, or whatever) that is completely understandable.
The problem you have here is that using this paradigm, you have no control over which thread gets woken, and so you have no enforceable guarantee that the threads will fire up again in the strict order that you started them in. You’d need to tell us why you want them in strict order, to understand what your goal with that is. Remember 10 divided by 4 doesn’t give an equal share to each person, but if there were 20 tickets for example then of course it would. In any case you can keep a tally of the number of tickets each thread buys if that is what you want.
I haven't really looked into your problem, but just glancing at it maybe you're looking for a cyclic barrier?
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CyclicBarrier.html

From the docs:
"A synchronization aid that allows a set of threads to all wait for each other to reach a common barrier point. "

You could setup the barrier, have all threads run one step, wait for each other to reach the barrier, then reset it and go again.  Might be what you need?
Doug, I don't think the order of threads arriving at the barrier can be directly controlled, which might mean back at square one.
Umm - if we want 4 threads to each always go in a specific order to perform a task, turns out that's super simple to solve:  remove 3 of the threads and make it single threaded :)

By definition multi-threaded code is all about having simultaneous work that is not guaranteed to be performed in order.  That's the point.  But we can guarantee that all 4 threads do one unit of work and then wait for each other before doing the next unit of work.  But if we're also guaranteeing that thread A goes first, then thread B, then C etc - that's a single threaded program.

Maybe the confusion here runs much deeper about the goals for using multiple threads?
This is the goal of my course assignment for threads to access the critical section - synchronized code. 4 tthreads will attempt code but only one will wins while others will wait And take turn to access it. ie. thread1 -> thread2 -> thread3 -> thread4 -> thread1 -> thread 2 ....

Wait and Notify are mandatory requirement but need to design a solution to solve it.....

Thx
Doug is saying it how it is - if the task is to have the threads operate in strict order, then - as I said above already - that cannot happen unless you are happy to allow the system to determine which thread gets woken up to restart. So the get a job done in strict successive order, you need to use a single thread, and have it run through the job sequentially.

But if your post above describes your assignment, then it looks like you have misinterpreted it, because nowhere does it say that the threads have to be restarted in the same strict order every time. I'm pretty sure that the assignment is based on the general principle of multithreading in Java, which is that any 'next' waiting thread will be fine for the system to choose to run. You have over-interpreted the parameters of your assignment, and are looking for a solution that is not available via the wait() notify() idiom.
Of course, you can also do this without using wait() notify() and synchronization, using other ways, one of which would be  :

import java.util.*;

class Buyer3 extends Thread {

   TicketPool pool;
      
     static TreeMap<String,Integer> names = new TreeMap<>();
     static int counter = 0;
     String[]  sA = {"John","Lily","Mary","Mike"};// should be {"John","Mike","Lily","Mary"};
     
   
   Buyer3(TicketPool pool)
   {
   
        this.pool = pool;
      
        for(String s : sA){names.put(s,0);}
   }
   
   public void run() //override run() here 
   {      
      //loop if availableTicket >0    
      
      while(pool.getTicket()>0){
                   
         
           if (pool.getTicket()>0) {
           
             
             if(!(counter == Arrays.binarySearch(sA,Thread.currentThread().getName()))){
             
                try{
                 Thread.currentThread().sleep(200);
                 continue;
               }catch (InterruptedException itro){
                 System.out.println("sleep is interrupted");
               }
            }
               
               
              // place order: print out "Thread xx reserved ticket --- #n"
              else{
                System.out.println(Thread.currentThread().getName()+" reserved ticket # "+ pool.getTicket());
         
                //delay 10ms: sleep(10)
                try{
                 Thread.currentThread().sleep(10);
               }
                catch (InterruptedException itro){
                 System.out.println("sleep is interrupted");
               }
         
                //payment finished: decrease availableTicket by 1 
                pool.setTicket(pool.getTicket()-1);
                counter++;
                //print the leftover tickets
                System.out.println(Thread.currentThread().getName()+" bought ticket # " + (pool.getTicket()+1) + ", ticket left: "+pool.getTicket());
               
                names.replace(Thread.currentThread().getName(),names.get(Thread.currentThread().getName()),names.get(Thread.currentThread().getName())+1);  
  
                 if(counter == names.size()){counter = 0;}
                  
             }
            }
            
            for(Map.Entry me : names.entrySet()){System.out.println(me);}
            
      }// go back to buy another once (while loop)   
      
   }// end of override run()

}

Open in new window


The order of the thread execution/reawakening is kept the same, (which is waht you were looking for), but the thread names in this case are sorted, which allows the binary search over the array to be effective. You can add some further manipulation to this idea if you really need the threads handled as per the exact order you used in your code.
btw, isn't the questions core more about scheduling concurrency to get a certain "load balancing" like round-robin?
ASKER CERTIFIED SOLUTION
Avatar of dpearson
dpearson

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Nice one Doug. ; )

I don't know if you tried my lump of code, ooi, but it does keep to the strict order of threads, albeit in the order they go into the Treemap.

;)
For its own sake, I'll post a robust, non-wait()/notify() non-synchronized version of code in which threads purchase tickets in strict rotation, and their totals are then passed back to the caller and aggregated in sorted order under their thread names.

import java.util.*;

class Buyer3 extends Thread {

   TicketPool pool;
      
     public static volatile TreeMap<String,Integer> names = new TreeMap<>();
     static volatile int counter = 0;
     private int mytag=0;
     private String nlk="";
     static volatile boolean done = false;
        
   Buyer3(TicketPool pool)
   {
   
        this.pool = pool;
      
        names.put(((Thread)this).getName(),0);
      
        nlk = names.lastKey();
             
                                    //System.out.println(names); //DEBUG
                                    //System.out.println(Thread.currentThread().getName());// = main //DEBUG
                                    
        mytag = Integer.parseInt(((Thread)this).getName().substring(7));
        
                                    //System.out.println("mytag is : "+mytag); //DEBUG
                                    //System.out.println("lastkey is : "+names.lastKey()); //DEBUG
       
   }
   
   public void run() //override run() here 
   {     
                 
      while(pool.getTicket()>0){
                   
           if (pool.getTicket()>0) {
             
             if(!(counter == mytag)){
             
                try{
                 Thread.currentThread().sleep(200);
                 continue;
               }catch (InterruptedException itro){
                 System.out.println("sleep is interrupted");
               }
            }
               
               
              // place order: print out "Thread xx reserved ticket --- #n"
              else{
                System.out.println(Thread.currentThread().getName()+" reserved ticket # "+ pool.getTicket());
         
                
                //delay ms: sleep()
                try{
                 Thread.currentThread().sleep(231);
               }
                catch (InterruptedException itro){
                 System.out.println("sleep is interrupted");
               }
         
                //payment finished: decrease availableTicket by 1 
                pool.setTicket(pool.getTicket()-1);
                
                counter++;
                
                //print the leftover tickets
                System.out.println(Thread.currentThread().getName()+" bought ticket # " + (pool.getTicket()+1) + ", ticket left: "+pool.getTicket());
                               
                names.replace(nlk,names.get(nlk),names.get(nlk)+1);
                
                TicketDemo3.results.put(Thread.currentThread().getName(),names.get(nlk));
  
                if(counter == names.size()){counter = 0;}
                  
             }
            }  
                                    
      }// go back to buy another once (while loop)  
                 
   }// end of override run()
}

Open in new window


import java.util.*;


public class TicketDemo3{
   
   
   public static Map<String, Integer> results = new TreeMap<>();
   
   public static void main (String[] args){
      // start 4 threads from a object of class Ticket that implements the "Runnable" interface
      TicketPool tickets = new TicketPool(10);
        
      // start 4 threads from the same object
      Buyer3 t1 = new Buyer3(tickets);
      Buyer3 t2 = new Buyer3(tickets);
      Buyer3 t3 = new Buyer3(tickets);
      Buyer3 t4 = new Buyer3(tickets);
      
      t1.setName("John");
      t2.setName("Mike");
      t3.setName("Lily");
      t4.setName("Mary");
                  
      t1.start();
       
      t2.start();
      
      t3.start();
      
      t4.start();  
          
                                                                //System.out.println("hello"); //DEBUG to show 'under the hood' precedence
      
      try{t4.join();}catch(Exception badjoin){badjoin.printStackTrace();}
      
      for(Map.Entry me : results.entrySet()){System.out.println(me.getKey()+" "+me.getValue());}
      
      System.gc();
      
   }
}

Open in new window



That's pretty cool Krakatoa.  This version uses "volatile" to help ensure the threading behavior is well behaved.

I would still urge anyone writing code today to also avoid the "volatile" keyword and use
AtomicReference
AtomicInteger
AtomicBoolean
for those places.  It's clearer and allows the implementations of those Atomics to improve over time (if the JDK wizards are able to do so), while volatile's behavior is defined in the language and won't be improving.

Also any code without "synchronize" on the ticket pool could potentially have issues around whether changes made by one thread are "visible" to another thread.  This all gets into the Java memory model and what data is allowed to be cached by one thread on the CPU and what data is not.  This code may actually be 100% correct (because the volatile references may enforce visibility rules) but as a general rule when you have 2 threads accessing shared data, you want something to use synchronize (or again - one of the Atomic classes which will handle that stuff for you) to make sure that the behaviors are thread safe.  It's commonly believed (incorrectly) that operations which modify integers are thread safe because you can't perform part of an integer operation (which is correct), but you can still have problems where other threads don't "see the change" for an arbitrary amount of time.  And how likely is that to happen on your laptop with 2 cores when you test it (very low) versus on that server you eventually run on with 16 cores (much higher), resulting in super hard to find bugs.

Ah - the fun with concurrency :)

That's a super-enlightening technical comment by you there Doug, well made and noted. Of course you are right in all senses about the need to use up-to-date API, and the pitfalls of not doing so. I guess any interest in the above code would be that it's a "exercise" if you like - (it shows, for one thing, that a thread's "given name" only materialises during its run() method, which is a little peculiar perhaps, and has to be fed back to the caller during that routine for it to persist and be useable, rather than being lost by its own termination).  

Concurrency on the slab !
;)
Thx