Hi Roger,

> On 8 Feb 2018, at 17:26, Roger Riggs <roger.ri...@oracle.com> wrote:
> 
> Hi Robin,
> 
> Saving useful information about failures during shutdown is useful.
> 
> Can the shutdown of JFR be put off? Is the use of shutdown hook still 
> necessary?

The part of JFR that manages recordings is written in Java, so the only other 
alternative I can see would be to introduce a hook at the very beginning of 
JVM_Halt. That one could call back into Java again and create the JFR shutdown 
hook thread from there instead. Not sure if there may be issues with the VM 
continuing to run a while longer after JVM_Halt is invoked, but I can’t see any 
obvious problem.

It is an interesting idea, such a mechanism would allow capturing events 
generated from other shutdown hooks or finalizers as well. Something like that 
would probably be a lot easier to try out after JFR is open source though.

Best regards,
Robin

> 
> Thanks, Roger
> 
> 
> On 2/8/2018 10:50 AM, Robin Westberg wrote:
>> Hi David,
>> 
>>> On 8 Feb 2018, at 04:28, David Holmes <david.hol...@oracle.com> wrote:
> ...
>>> I had an initial look through this.
>>> 
>>> To be honest I don't like it. We seem to have to record little bits of 
>>> information all over the place just so they can be reported by the shutdown 
>>> event. It seems untidy. :(
>>> 
>>> Can you rename _starting_thread to _main_thread please. The use of 
>>> "starting" in thread.hpp/cpp is really a naming anomaly. The main thread is 
>>> the thread that loads the JVM. And that would be consistent with 
>>> set_exception_in_main_thread.
>>> 
>>> Though why do we care if the main thread exited with an unhandled 
>>> exception? And if we do care, why do we only care when the shutdown reason 
>>> is ""No remaining non-daemon Java threads"?
>>> 
>>> I don't like the need to add os::get_abort_exit_code. Do we really need it? 
>>> What useful information does it convey?
>> 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.
>> 
>> 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.
>> 
>>> 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).
>> 
>> 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
> 

Reply via email to