JDK-8154017 : Shutdown hooks are racing against shutdown sequence, if System.exit()-calling thread is interrupted
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.lang
  • Affected Version: 7,8,9
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2016-04-11
  • Updated: 2018-05-14
  • Resolved: 2016-06-24
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u172Fixed 9 b125Fixed
Description
This happens because ApplicationShutdownHooks.runHooks joins the shutdown hooks. This works normally if interrupt status is not set in advance. But if it does, we exit the loop early:

    static void runHooks() {
        Collection<Thread> threads;
        synchronized(ApplicationShutdownHooks.class) {
            threads = hooks.keySet();
            hooks = null;
        }

        for (Thread hook : threads) {
            hook.start();
        }
        for (Thread hook : threads) {
            try {
                hook.join();
            } catch (InterruptedException x) { }  // oops, no join for you, falling through...
        }
    }

...and already started shutdown hooks begin to race against the rest of the shutdown sequence. Notably, this may lead to premature VM shutdown, with shutdown hooks not fully executed, as test demonstrates.

ApplicationShutdownHooks is called from the thread that originally called System.exit:

j java.lang.ApplicationShutdownHooks.runHooks()V+113
j  java.lang.ApplicationShutdownHooks$1.run()V+0
j  java.lang.Shutdown.runHooks()V+100
j  java.lang.Shutdown.sequence()V+26
j  java.lang.Shutdown.exit(I)V+95
j  java.lang.Runtime.exit(I)V+14
j  java.lang.System.exit(I)V+4
j  Main.main([Ljava/lang/String;)V+20

Full jtreg test:
  http://cr.openjdk.java.net/~shade/8154017/ShutdownInterruptedMain.java
Comments
Thanks Aleksey, I'll post a review soon. As with the existing code, interrupt status is not retained, just ignored. This code is implementing System.exit, so should not be an issue.
23-06-2016

Yes, looks fine. As long as we wait for all hooks to complete (which mean we have to ignore interrupts), the race is not happening.
23-06-2016

Suggested fix: diff --git a/src/java.base/share/classes/java/lang/ApplicationShutdownHooks.java b/src/java.base/share/classes/java/lang/ApplicationShutdownHooks.java --- a/src/java.base/share/classes/java/lang/ApplicationShutdownHooks.java +++ b/src/java.base/share/classes/java/lang/ApplicationShutdownHooks.java @@ -90,21 +90,25 @@ /* Iterates over all application hooks creating a new thread for each * to run in. Hooks are run concurrently and this method waits for * them to finish. */ static void runHooks() { Collection<Thread> threads; synchronized(ApplicationShutdownHooks.class) { threads = hooks.keySet(); hooks = null; } for (Thread hook : threads) { hook.start(); } for (Thread hook : threads) { - try { - hook.join(); - } catch (InterruptedException x) { } + while (true) { + try { + hook.join(); + break; + } catch (InterruptedException x) { + } + } } } }
23-06-2016