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>> 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
