What did I do wrong in the following piece of code that caused threads to never terminate?

Question

I have the following piece of code which I expected to print "DONE" at the end. But when I ran, "DONE" was never printed and the JVM in fact never terminated.

What did I do wrong?

// File: Simple.java
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

public class Simple {


    public static void main(String[] args) throws Exception{

        doTest(3);

    }

    private static void doTest(final int times) {

        ScheduledExecutorService tp = Executors.newScheduledThreadPool(times);

         Thread[] runnables = new Thread[times];
         for (int i = 0; i < runnables.length; ++i) {
             runnables[i] = new Thread(new MyRunnable());
         }

         // schedule for them all to run
         for (Thread t : runnables) {
             tp.schedule(t, 1, TimeUnit.SECONDS);
         }


         try {
            tp.awaitTermination(Long.MAX_VALUE, TimeUnit.SECONDS);
            System.out.println("DONE!");
         }catch (InterruptedException e) {
            e.printStackTrace();
        }

     }


    static class MyRunnable implements Runnable {
        @Override
        public void run() {
            System.out.println("hello world");
        }

    }

}

Answer 1

Executors.newScheduledThreadPool(times) uses Executors.defaultThreadFactory() for its ThreadFactory.

Here is the documentation

When a Java Virtual Machine starts up, there is usually a single non-daemon thread (which typically calls the method named main of some designated class). The Java Virtual Machine continues to execute threads until either of the following occurs:

• The exit method of class Runtime has been called and the security manager has permitted the exit operation to take place.
• All threads that are not daemon threads have died, either by returning from the call to the run method or by throwing an exception that propagates beyond the run method.

So, here is what you had; but I changed 3 things.

• Added the shutdown hook
• Made the schedule delayed by an extra second for each to demo that the shutdown is being called even before some of the threads are being called to run by the scheduler.

• Lastly, you were telling it to wait forever using the Long.MAX. But I think you were doing it because the shutdown feels like it would shutdown now. But it won't.

  public static void main(String[] args) throws Exception {
  
      doTest(3);
  
  }
  
  private static void doTest(final int times) {
  
      ScheduledExecutorService tp = Executors.newScheduledThreadPool(times);
  
      Thread[] runnables = new Thread[times];
      for (int i = 0; i < runnables.length; ++i) {
          runnables[i] = new Thread(new MyRunnable());
      }
  
      // schedule for them all to run
      int i = 1;
      for (Thread t : runnables) {
          tp.schedule(t, i++, TimeUnit.SECONDS);
      }
  
  
      System.out.println("Calling shutdown");
      tp.shutdown();
  }
  
  
  static class MyRunnable implements Runnable {
      @Override
      public void run() {
          System.out.println("hello world");
  
      }
  
  }
  

Hope that answers your question. Now, you're kinda duplicating stuff.

If you are going to use the executerservice, you should just tell it to schedule stuff for you.

public static void main(String[] args) throws Exception {

    doTest(3);

}

private static void doTest(final int times) {

    ScheduledExecutorService tp = Executors.newScheduledThreadPool(times);

    for (int i = 0; i < times; ++i) {
        tp.schedule(new MyRunnable(), 1, TimeUnit.SECONDS);
    }



    System.out.println("Calling shutdown");
    tp.shutdown();
}


static class MyRunnable implements Runnable {
    @Override
    public void run() {
        System.out.println("hello world");

    }

}

Answer 2

There are two things that you're doing wrong here.

First off, if you're using an ExecutorService, you shouldn't then also be creating your own threads. Just submit Runnables to the executor directly - the executor service has its own collection of threads, and runs anything you submit on its own threads, so the threads you created won't even get started.

Second, if you're done with an ExecutorService, and are going to wait until it's terminated, you need to call shutdown() on the executor service after you submit your last job.

Answer 3

You forgot to shutdown your ExecutorService:

tp.shutdown(); // <-- add this
tp.awaitTermination(Long.MAX_VALUE, TimeUnit.SECONDS);

Also I should mention that creating Threads to use them as Runnables is not only meaningless, but can be misleading as well. You should really use Runnable instead of Thread for runnables variable.

Runnable[] runnables = new Runnable[times];