Java Shutdown Hooks

Question

Using IntelliJ IDE, java 1.8, lang level 6 in IDE (and whatever is default at the command prompt).

Compiling and running from commandline (Windows 7) with:

javac -cp . Main.java
java -cp . Main

Given the following code, what I'd like to do is shut down all of the running threads in the app (and maybe I don't need to do so, if the OS or JVM does it for me). I have a thread MyThread, which in turn, holds a list of threads it's managing (and when interrupted by the main, will send an interrupt to each thread in its list).

When I run the code, I get the output Keeping busy. printed every 2s. When I press CTRL-C, the shutdown hook runs (I put println messages in there to confirm) but that is all of the output I see. I never see the Cancelled. nr the end of the main()... so I must also presume that the Cancel method is also not being run.

So, first of all, do I need to care about this? Am I okay with just looping (in main) on isInterrupted and letting the program cease? How do all of the sub-threads get cancelled?

import java.util.HashMap;

class MyThread extends Thread {}

public class Main
{
    private static volatile boolean running = true;
    // List of active threads.
    private HashMap<String, Thread> threads = new HashMap<String, Thread>();

    public static void main(String[] args) throws InterruptedException
    {
        // Get an instance of the Main class.
        Main myApp = new Main();

        Runtime.getRuntime().addShutdownHook(new Thread()
        {
            public void run()
            {
                running = false;
            }
        });

        // Loop until system shutdown.
        while (running && !Thread.currentThread().isInterrupted())
        {
            myApp.CreateMyThread();

            // In real code, I have a wait() here, rather than the following lines.

            System.out.println("Keeping busy.");
            Thread.sleep(2000);
        }

        System.out.println("Cancelled.");

        // Kill all threads.
        myApp.Cancel();

        System.exit(0);
    }

    private void Cancel()
    {
        for (Thread t : threads.values())
        {
            t.interrupt();
        }
    }

    private void CreateMyThread()
    {
        if (threads.get("mythread") instanceof MyThread)
        {
            // Already exists.
            return;
        }

        MyThread t = new MyThread();
        t.start();
        threads.put("mythread", t);
    }
}

EDIT:

So, I changed my code as follows. Created a Threadlist class containing the hashmap, with methods using keyword 'synchronized' to add/remove a thread and to cancel all threads. The main contains a static instance of this class. My shutdown handler then calls the CancelThreads method to shut down all running threads. Question: is this truly thread-safe, ie: is having synchronized on methods actually blocking one method from being run when another is in progress?

import java.util.HashMap;

class MyThread extends Thread {}

class Threadlist
{
    // List of threads.
    private HashMap<String, Thread> threads = new HashMap<String, Thread>();

    public synchronized void AddThread(String name, Thread thread)
    {
        threads.put(name, thread);
    }

    public synchronized Thread GetThread(String name)
    {
        return threads.get(name);
    }

    public synchronized void CancelThreads()
    {
        for (Thread t : threads.values())
        {
            System.out.printf("Killing a thread by the name of '%s'\n", t.getName());
            t.interrupt();
        }
    }
}

public class Main
{
    private static volatile boolean running = true;
    // List of active threads.
    private static Threadlist threads = new Threadlist();

    public static void main(String[] args) throws InterruptedException
    {
        // Get an instance of the Main class.
        Main myApp = new Main();

        Runtime.getRuntime().addShutdownHook(new Thread()
        {
            public void run()
            {
                threads.CancelThreads();
                running = false;
            }
        });

        // Loop until system shutdown.
        while (running && !Thread.currentThread().isInterrupted())
        {
            myApp.CreateMyThread();

            // In real code, I have a wait() here, rather than the following lines.

            System.out.println("Keeping busy.");
            Thread.sleep(2000);
        }

        System.out.println("Cancelled.");

        System.exit(0);
    }

    private void CreateMyThread()
    {
        if (threads.GetThread("mythread") instanceof MyThread)
        {
            // Already exists.
            return;
        }

        MyThread t = new MyThread();
        t.start();
        threads.AddThread("mythread", t);

}

}

I had also begun creating a ShutdownHook class, extending Thread, which would have accepted main's threads object via the constructor and then cancelled the threads in the run() method, but I couldn't see how to change the main->running value at the same time, hence, my approach above.

Answer 1

Once shutdown hooks finish running the VM halts (there is actually another step in between but not relevant for this discussion).

The key here is your shutdown hook needs to live long enough for other thread to clean up, you can do this by having your shutdown hook call Thread.join(long) on the other thread, with a timeout. This will allow the shutdown hook to end after the other thread or when the specified timeout is exceeded.

Blocking the shutdown processes is usually considered to be not user friendly. Your shutdown hook should wake up or signal the other thread before joining on it, how this is done is up to you (interrupt(), wait()/notifyAll() pairing, or using a class from java.util.concurrent package.

Example:

    final Object lock = new Object(); //Lock object for synchronization.
    final Thread mainThread = Thread.currentThread(); //Reference to the current thread.

    Runtime.getRuntime().addShutdownHook(new Thread()
    {
        public void run()
        {
            try{
                synchronized(lock){
                  running = false;
                  lock.notifyAll();
                }

                mainThread.join(4000);
            }catch(InterruptedException){
               //Interrupted.
            }
        }
    });

    synchronized(lock){
        while (running && !Thread.currentThread().isInterrupted())
        {
            myApp.CreateMyThread();

            System.out.println("Keeping busy.");
            lock.wait(2000); 
        }
     }

There are some issues with the above example, a big one being that wait() is not useful as a timer (wait() is allowed to spuriously wake up), so if you really need to have it be every 2 seconds the logic is more complicated. However, this outlines the basics of getting it to work.

Answer 2

Your shutdown hook is a separate Thread that is kicked off when shutdown occurs (hopefully, no guarantee). Your main thread, meanwhile, is still progressing, but it is about to get killed. This MIGHT work, but the interrupt probably won't give you 2 seconds you have there in your code before it kills the main thread.

Instead, you should share the list of spawned threads with your shutdown hook and have that hook attempt to interrupt/shutdown the spawned threads directly.