RFR: 8041626: [Event Request] Shutdown reason

[David Holmes]

Hi Robin,

On 10/02/2018 1:41 AM, Robin Westberg wrote:
> Hi David,
> 
>> On 9 Feb 2018, at 05:22, David Holmes <david.holmes at oracle.com 
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>> Hi Robin,
>>
>>> Right, almost all the runtime changes are made in order to try to 
>>> figure out what the process exit code from the launcher will 
>>> eventually be. For example, the launcher returns 1 if the main thread 
>>> exited with an unhandled exception, but 0 otherwise. But I actually 
>>> agree that this information is probably only of marginal use (it 
>>> could always be captured from wherever Java is launched if someone 
>>> really wants it), so it is perhaps not worth the trouble.
>>
>> Yes and that particular example of launcher behaviour is an archaic 
>> throwback to C programming - where end of "main" means end of the 
>> program - IMHO. I don't think it is of interest - and certainly not 
>> worth jumping through so many hoops to make the info available.
>>
>>> Discussed it a bit with Erik Gahlin, and perhaps the best option here 
>>> is to simply remove the status code field from the event, that would 
>>> simplify things and make the code you mention above go away.
>>
>> That sounds good to me. :)
>>
>>>> It is unfortunate that you need to add beforeHalt to deal with the 
>>>> event mechanism itself being turned off (?) by a shutdown hook. That 
>>>> would seem to potentially lose a lot of event information given 
>>>> hooks can run in arbitrary order and execute arbitrary Java code. 
>>>> And essentially you end up recording the initial reason shutdown 
>>>> started, though potentially it could end up terminating for a 
>>>> different reason.
>>> In this case I think it actually conveys useful information if you 
>>> are trying to diagnose an unexpected shutdown. It should be useful to 
>>> find the initial request of an orderly shutdown, even if something 
>>> else happens during the shutdown sequence like a finalizer calling 
>>> exit (in which case you could possibly end up with two shutdown 
>>> events, but both may contain interesting information).
>>
>> Yes that is useful. But I find the need to code things the way they 
>> are, unfortunate. But given current constraints, so be it.
> 
> Okay, I’ve removed the code related to the status field, certainly makes 
> the change a bit less intrusive.
> 
> Updated webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/
> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/

This looks much cleaner/neater to me - thanks.

Tool Nit: the git-webrev tool you use has an annoying quirk compared to 
our own webrev tool - it first reports the number of files changed even 
when it is reporting on an individual file! I'm used to seeing a first 
number that represents number of lines changes - and the "1" in this 
case throws me as I like to quickly look at the size of the change for 
each file when deciding what order to look at them. :(

Thanks,
David

> Best regards,
> Robin
> 
>>
>> Thanks,
>> David
>>
>>> Best regards,
>>> Robin
>>>> Let's see what others think ...
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 8/02/2018 1:18 AM, Robin Westberg wrote:
>>>>> Hi all,
>>>>> Please review the following change that adds an event-based tracing 
>>>>> event that is generated when the VM shuts down. The intent is to 
>>>>> capture shutdowns that occur after the VM has been properly 
>>>>> initialized (as initialization problems would most likely mean that 
>>>>> the tracing framework hasn’t been properly started either).
>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8041626
>>>>> Webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00/
>>>>> Testing: hs-tier1,hs-tier2,jdk-tier1,jdk-tier2
>>>>> Best regards,
>>>>> Robin
>