[11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[mandy chung]

On 2/21/18 6:08 PM, David Holmes wrote:
> Hi Mandy,
>
> tl;dr: I think this is now good to go.
>
> On 22/02/2018 5:58 AM, mandy chung wrote:
>> Here is the updated webrev:
>> http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.04/
>>
>> I added some comments to clarify the implementation.
>
> src/java.base/share/classes/java/lang/Shutdown.java
>
>  96                 if (VM.isShutdown()|| slot <= currentRunningHook)
>
> Nit: need space after ()
>
>

Fixed.

>  113 private static void runHooks() {
>  114         synchronized (lock) {
>  115             /* Guard against the possibility of a daemon thread 
> invoking exit
>  116              * after DestroyJavaVM initiates the shutdown sequence
>  117              */
>  118             if (VM.isShutdown()) return;
>  119         }
>
> I think this is actually impossible to hit, but that's a separate 
> cleanup.

When T1 is calling runHooks, T2 calls exit and blocks on Shutdown.class, 
once T1 releases the lock if VM does not halt yet, T2 will enter this 
method.
>
>  139         // set shutdown state
>  140         VM.shutdown();
>
> Just an observation that we consider "shutdown" to be after all system 
> shutdown hooks are run. So this (contrary to what I wrote in the CSR) 
> narrows the window in which a concurrent exit(-1) would trigger an 
> immediate halt rather than a hang. Which in turn strengthens the 
> argument for just dropping that behaviour.

If an application hook calls exit(0), it will hang (not the thread 
holding the Shutdown class lock).  Do you want to file an issue to track 
that?

>
> ---
>
> src/java.base/share/classes/jdk/internal/misc/VM.java
>
>  103     public static void shutdown() {
>  104         initLevel(SYSTEM_SHUTDOWN);
>  105     }
>
> Doing this is exactly what I meant by my previous comments.
>

Good.

Thanks for the thorough review.
Mandy