RFR (S): 8222534: VerifyBeforeExit is not honored when System.exit is called

[coleen.phillimore at oracle.com]

David, Thank you for the writeup of the exit paths.

The reason I added this test with JDK-9074355 is because taking the 
Heap_lock in destroy_vm prevented a crash in VerifyBeforeExit.

http://cr.openjdk.java.net/~coleenp/2019/8074355.06/webrev/test/hotspot/jtreg/runtime/Shutdown/ShutdownTest.java.html

I wonder if you need to take the Heap_lock in the VM_Exit path as well.

Coleen

On 4/29/19 11:54 PM, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8222534
> Webrev: http://cr.openjdk.java.net/~dholmes/8222534/webrev/
>
> Stefan noticed that VerifyBeforeExit was not honoured when a Java 
> application terminates via System.exit.
>
> Examination of the exit code sequences revealed four differences 
> between an exit due to the last non-daemon thread terminating (handled 
> via jni_DestroyJavaVM) and a call to System.exit() (handled via 
> JVM_Halt()). There are four missing actions on the System.exit() path:
>
> - No processing of VerifyBeforeExit
> - No XML logging before exit
> - No LogConfiguration::finalize()
> - No IdealGraphPrinter::clean_up()
>
> The first three have now been added at the appropriate point. 
> VerifyBefore exit was the main omission. The compiler team  (i.e. 
> Vladimir K.) indicated they'd also like the XML logging. And the 
> LogConfiguration::finalize while possibly not essential avoids any 
> doubt as to whether buffered log output may get lost.
>
> The IdealGraphPrinter::cleanup was deemed unnecessary due to the fact 
> the process is being blown away anyway.
>
> The bug report contains a lot of details on the exit sequences 
> including a side-by-side comparison in the attached pdf, showing the 
> relative positioning of each action and that the correct order has 
> been maintained.
>
> The vm_exit() code affects a number of "abrupt" exit paths in the VM, 
> not just JVM_Halt, and this is discussed in the bug report as well. In 
> short the addition of the missing actions should not cause any issues.
>
> Testing:
>  - some manual checking of exit paths and whether new code was executed
>  - all hotspot/jtreg/gc tests with -XX:+VerifyBeforeExit added
>  - mach5 tiers 1-3
>
> Thanks,
> David