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

[mandy chung]

Hi David,

I think I'm clear on the implementation now (my mistake that I neglected 
ApplicationShutdownHooks).  Shutdown class keeps the "system shutdown 
hooks".  ApplicationShutdownHooks is one system hook that starts all 
application shutdown hooks and waits until they finish.   
java.io.Console and DeleteOnExitHook are two other system shutdown hooks 
to restore console echo state or support deleteOnExit.

The system shutdown hooks are all run in the thread initiating the 
shutdown that holds the Shutdown.class.

On 2/20/18 10:27 PM, David Holmes wrote:
> Hi Mandy,
>
> On 21/02/2018 5:57 AM, mandy chung wrote:
>> Hi David,
>>
>> I reworked the change in Shutdown class and uses jdk.internal.misc.VM 
>> to maintain the shutdown state, either in progress or shutdown (i.e. 
>> all shutdown hooks have been started).
>>
>> What do you think this revised version:
>> http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.03/
>
> It confuses me a bit. On the one hand I wasn't sure why we needed to 
> bring the VM class into this, then on the other hand it perhaps made 
> sense to have VM track shutdown states as well as initialization states.
>

The latter is my motivation and I think it's a good cleanup to have VM 
state to indicate it's shutdown.

> But I'm not sure about the two-phase "shutdown" state - I think I'd 
> probably prefer a "shutdown initiated" state and a "shutdown complete" 
> state. Then no need to duplicate the constant values for IN_PROGRESS 
> and SHUTDOWN (which now need to be kept in sync with changes to the VM 
> class). It would also simplify the shutdown() logic.
>

Shutdown::add method can be used to register a system hook while 
shutdown is in progress.  Actually currentRunningHook can be used to 
guard if Shutdown::add should reject to add a new hook.  I updated the 
webrev and no longer needs the two phases.

> Also shutdown(level) should be using initLevel(value) so that it fits 
> in with the existing awaitInitLevel() logic and locking strategy! 
> Someone may want to wait for shutdown in the future. That also deals 
> with the locking issue in the Shutdown class because you don't need to 
> use "synchronized(lock)" because runHooks is only called within 
> "synchronized(Shutdown.class)". [To be fair the existing locking 
> strategy seems confused to me as well - or at least it confuses me.]

I agree but I tried not to touch the existing locking strategy as it 
should be a separate changeset.  I prefer to add VM::shutdown method
>
> I also now realize that the changes I suggested for the Runtime.exit 
> docs was incorrect. The existing docs state that we only halt if hooks 
> have already been run - which corresponds to the old and new code. I, 
> for some reason that escapes me, claimed we only cared if the hooks 
> had been started, not completed - but that is inconsistent with old 
> spec and the implementation.
>

I confused myself too.  A small change will fix it:

87 * <p> If this method is invoked after all shutdown hooks have already
88 * been run and the status is nonzero then this method halts the
89 * virtual machine with the given status code. Otherwise, this method



Mandy

> So apologies but what started out as a seemingly simple code removal, 
> has become a lot more complicated, and confusing to me.
>
> Thanks,
> David
> -----
>
>> On 2/15/18 9:14 PM, David Holmes wrote:
>>>
>>> All other updates seem okay. I did have one further thought - in 
>>> Runtime does this change:
>>>
>>>       public void runFinalization() {
>>> ! SharedSecrets.getJavaLangRefAccess().runFinalization();
>>>       }
>>>
>>> affect the classloading/initialization order at all?
>>
>> Runtime::runFinalization is not called by the system code.  So it 
>> won't be invoked during startup and hence won't change the 
>> classloading order during startup.
>>
>> Mandy