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

[David Holmes]

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.

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.

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

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