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

[David Holmes]

Hi Mandy,

On 22/02/2018 12:28 PM, mandy chung wrote:
> 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.

Ah! T1 is executing destroyJavaVM - and of course releases the lock 
eventually. Got it.

>>
>>  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?

If an application hook calls exit(0) it will block on the Shutdown.class 
lock which then hangs the whole shutdown process as the thread holding 
that lock is doing a join() on the hook thread - logical deadlock.

But that wasn't my point at all. I just think this whole "calling exit 
with non-zero causes immediate halt" logic is meaningless now there are 
no finalizers running (and potentially calling exit).

Cheers,
David
-------

>>
>> ---
>>
>> 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
>