Hi David, > On 9 Feb 2018, at 05:22, David Holmes <david.hol...@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/ <http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/ <http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/> 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