Program never ends but Thread Pool claims to shut down

Question

I got an issue with a multi-threaded program.

I need to simulate a lot of people trying to book the same flight at the same time, using no locks.

So I made an ExecutorService and used that to pool threads so I can have many simultaneous attempt going at once.

The problem is though, the program sort of reaches the end and right before it prints out all the results, it just sits there and runs forever. I've tried to go into all the other classes that utilize a connection to a database and simply close them off manually. No luck.

package dbassignment4;

   import java.util.ArrayList;
   import java.util.List;
   import java.util.concurrent.ExecutionException;
   import java.util.concurrent.ExecutorService;
   import java.util.concurrent.Future;
   import java.util.concurrent.LinkedBlockingQueue;
   import java.util.concurrent.RejectedExecutionException;
   import java.util.concurrent.ThreadPoolExecutor;
   import java.util.concurrent.TimeUnit;

   /**
    *
    * @author Vipar
    */
   public class Master{

   private static final int POOL_SIZE = 50;
   public static boolean planeIsBooked = false;
   /**
    * @param args the command line arguments
    */
   public static void main(String[] args) {

       int success = 0;
       int seatNotReserved = 0;
       int seatNotReservedByCustomerID = 0;
       int reservationTimeout = 0;
       int seatIsOccupied = 0;
       int misc = 0;

       HelperClass.clearAllBookings("CR9");
       Thread t = new Thread(new DBMonitor("CR9"));
       long start = System.nanoTime();
       //HelperClass.clearAllBookings("CR9");
       ExecutorService pool = new ThreadPoolExecutor(
               POOL_SIZE, POOL_SIZE,
               0L,
               TimeUnit.MILLISECONDS,
               new LinkedBlockingQueue<Runnable>(POOL_SIZE));
       int threadsStarted = 0;
       List<Future<Integer>> results = new ArrayList<>();
       long id = 1;
       t.start();
       while(planeIsBooked == false) {
           try {
           Future<Integer> submit = pool.submit(new UserThread(id));
           results.add(submit);
           } catch (RejectedExecutionException ex) {
               misc++;
               continue;
           }
           threadsStarted++;
           id++;
       }
       pool.shutdownNow();

       int count = 0;
       for(Future<Integer> i : results) {
           try {
               switch(i.get()) {
                   case 0:
                       // Success
                       success++;
                       break;
                   case -1:
                       // Seat is not Reserved
                       seatNotReserved++;
                       break;
                   case -2:
                       // Seat is not Reserved by Customer ID
                       seatNotReservedByCustomerID++;
                       break;
                   case -3:
                       // Reservation Timeout
                       reservationTimeout++;
                       break;
                   case -4:
                       // Seat is occupied
                       seatIsOccupied++;
                       break;
                   default:
                       misc++;
                       // Everything else fails
                       break;
               }
           } catch (ExecutionException | InterruptedException ex) {
               misc++;
           }
           count++;
           System.out.println("Processed Future Objects: " + count);
           // HERE IS WHERE IT LOOPS
       }

Here is the rest of the code it doesn't execute right after:

long end = System.nanoTime();
long time = end - start;
System.out.println("Threads Started: " + threadsStarted);
System.out.println("Successful Bookings: " + success);
System.out.println("Seat Not Reserved when trying to book: " + seatNotReserved);
System.out.println("Reservations that Timed out: " + reservationTimeout);
System.out.println("Reserved by another ID when trying to book: " + seatNotReservedByCustomerID);
System.out.println("Bookings that were occupied: " + seatIsOccupied);
System.out.println("Misc Errors: " + misc);
System.out.println("Execution Time (Seconds): " + (double) (time / 1000000000));
}
}

Can you spot the issue? I put in a comment where the code stops running.

Never **ever** do this: `planeIsBooked == false`. `planeIsBooked == (true == false)` is much more readable. Or `planeIsBooked == (1 == 2 == true)`. You get the point. — Boris the Spider, May 03 '14 at 19:48
I don't see you stop `Thread t`. Also use the factory methods in [`Executors`](http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html) rather than the constructor. — Boris the Spider, May 03 '14 at 19:50
@salyh Hello, I am the one who originally wanted to submit the question, but I've asked too much today. Basically we close the pool because we now achieved the goal of booking every seat on the plane. — OmniOwl, May 03 '14 at 19:50
Further `planeIsBooked` is not `volatile` so if you update it from your other threads, this change **will not be visible**. Basically, this code is all wrong... — Boris the Spider, May 03 '14 at 19:52

score 0 · Answer 1 · answered May 03 '14 at 19:50

0

When planeIsBooked become true, looks like your planeIsBooked will never be initialize to true in the while loop. So make sure your loop is not infinite.

answered May 03 '14 at 19:50

user3579257

21
3

The DBMonitor checks planeIsBooked separately and refreshes that variable to true (which works). – OmniOwl May 03 '14 at 19:50

score 0 · Answer 2 · answered May 03 '14 at 19:53

0

first thing is while(planeIsBooked == false) evaluates to true always because

planeIsBooked = false always , nowhere its initialized to true.

so how come your while loop condition becomes false and come out?

set inside while loop somewhere planeIsBooked = true to come out of while loop.

answered May 03 '14 at 19:53

Karibasappa G C

2,686
1
18
27

have you debugged to check the value of planeIsBooked setting false ? because in multithreaded environment, to make variable value consistent across the thread we use volatle but it seems you have not done it...so it might be adding infinite values to arraylist results..please check whether while loop runs nfinitely ? – Karibasappa G C May 03 '14 at 20:03

score 0 · Answer 3 · answered May 03 '14 at 19:54

Couple of Things :

First , After you call pool.shutdownNow(); - you are proceeding to try fetching the results straight away. The call shutDownNow() is not blocking and is not a definite indication that pool is stopped. For that - you should call pool.awaitTermination().

Second, It is not clear what you mean by your comment -

// HERE IS WHERE IT LOOPS

This is under a loop - and looking at the loop - if there is an exception thrown in the switch case - then it will go into the catch - ignore it and loop. Did you check for exceptions ?

score 0 · Answer 4 · edited May 23 '17 at 12:20

Go through this answer for why you should declare static variables as volatile in multi thread environment.

Even though , your static variable is volatile , the following lines is dangerous

while(planeIsBooked == false) {
        Future<Integer> submit = pool.submit(new UserThread(id));
   }

Consider your booking flight taking avg 2 secs . And you have about 300 seats ( assumption). Your planeIsBooked field would become true after 600 seconds ( if its running in single thread). With 50 size pool it would run in 12 seconds.

With the above assumption , your loop would run 12 seconds. Now , think about how many time the submit request statement executed ? i times. Even though you have just 300 seats , you might given appox more minimum million requests with in 12 seconds.

Thus, think about the number Jobs in queue before calling Shutdown now() . THis is not the right way of terminating loop

If you know the max size of your flight seat , why dont you use it in for loop ( may be externalize the parameter in for loop , instead of variable to hold

Program never ends but Thread Pool claims to shut down

4 Answers4