Hi David, > On 12 Feb 2018, at 08:07, David Holmes <david.hol...@oracle.com> wrote: > > Hi Robin, > > On 10/02/2018 1:41 AM, Robin Westberg wrote: >> Hi David, >>> On 9 Feb 2018, at 05:22, David Holmes <david.hol...@oracle.com >>> <mailto:david.hol...@oracle.com> <mailto:david.hol...@oracle.com >>> <mailto: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/ >> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/ > > This looks much cleaner/neater to me - thanks.
Thanks for reviewing! > 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. :( Good point, I’ll make sure the git-webrev tool is updated to more closely match the output of the standard webrev tool. Best regards, Robin > > 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
