JDK-8220787 : Create new switch to redirect error reporting output to stdout or stderr
  • Type: CSR
  • Component: hotspot
  • Sub-Component: runtime
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 13
  • Submitted: 2019-03-18
  • Updated: 2022-11-29
  • Resolved: 2019-03-27
Related Reports
CSR :  
Description
Summary
-------

Add two new switches to redirect error reporting output to stdout or stderr, respectively.


Problem
-------

In certain container environments the local file system is ephemeral - its content does not survive the death of the container. In those environments post-mortem analyzing crashes can be challenging because core files and hs-err files are lost. 

Since stdout and stderr of the VM process are usually catched in some sort of logging system, hs-err files could be written to stdout or stderr instead, increasing chances of meaningful post-mortem analysis.


Solution
--------

Two new switches are added which, when specified, redirect the content that would have normally written to the hs-err file to stdout or stderr instead.

Specification
-------------

Two new boolean switches are proposed, "ErrorFileToStdout" and "ErrorFileToStderr", respectively. If specified, if a fatal error occurs which normally would result in an hs-err file written to the file system, the content of that hs-err file would be written to stdout or stderr instead.

If both "ErrorFileToStdout" and "ErrorFileToStderr" are specified on the command line, the switch specified last overrides the prior switch.


If specified in conjunction with -XX:ErrorFile, -XX:ErrorFile will be ignored.

    +  product(bool, ErrorFileToStdout, false,                                   \
    +          "If specified, error data will be written to stdout.")            \
    +                                                                            \
    +  product(bool, ErrorFileToStderr, false,                                   \
    +          "If specified, error data will be written to stderr.")            \
    +                                                                            \

----

Clarification:

Post Mortem error data come in two parts: a first, abbreviated section (the '#' section) is always dumped to stdout. A second, full report usually goes to the error file (default name hs_err...).

This proposal only concerns the second part and does not affect the first part. We will continue to print out the abbreviated report to stdout first, unless ErrorFileToStdout is given (so both parts are to be printed to stdout), in which case the first part is suppressed to avoid redundant information.

----

Alternatives considered and rejected:

a) We briefly considered reusing the ErrorFile switch, giving it two special values for stdout, stderr respectively, e.g."-XX:ErrorFile=(STDOUT|STDERR)". Two problems with that:
- Existing installations may already use this value, expecting post mortem files with that name
- Accidentally using this form on a VM which is too old would create a file with that name where one would expect output to stdout, stderr
Compounded are these points by the fact that one only will notice something is wrong when the VM crashes, not before. 

b) An alternative form of "-XX:ErrorFileRedirectTo=(STDOUT|STDERR)" was proposed, which would have resulted in one one, not two switches. This was rejected because -XX:+ErrorFileToStderr/ToStdout is more in line with existing switches DisplayVMOutputToStderr/Out.






Comments
Thanks for the additional background Thomas and David; moving to Approved.
27-03-2019

did so. Thanks David & Joe. ..Thomas
27-03-2019

Hi Thomas, Nothing has changed that would require a re-review. Please move this to Finalized. Thanks, David
27-03-2019

Hi Joe, what David said. I updated the description to describe the alternatives considered and rejected. I checked https://wiki.openjdk.java.net/display/csr to find out what exactly would be necessary to move from Provisional to Finalized. Do all reviewers need to repeat the review? Thanks, Thomas
27-03-2019

Hi Joe, the current syntax was suggested by me to mirror what we already have with the DisplayVMOutputToXXX flags. I thought consistency here would be a good thing. Options were initially discussed here: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-March/033160.html Using the existing -XX:ErrorFile=<path> would have been ideal from a flag perspective but unfortunately it makes the implementation much more complicated to special-case stderr and stdout usage. Thanks.
27-03-2019

I have not reviewed the full email review of the change. Looking toward reducing the number of HotSpot flags, was an alternative like ErrorFileTo:{StdOut, StdErr} considered? In other words, a single option with a StdOut vs StdErr argument. Moving to Provisional. Before the change is pushed, the request should be Finalized and then reviewed again by the CSR for Approval.
27-03-2019

I did not know that but sure, no problem. When in doubt behave in an unsurprising manner :)
20-03-2019

Thank you for making that change. HotSpot has a very long history of "last option wins".
20-03-2019

You'd have to do something manual in arguments.cpp to prefer stderr to stdout. This seems straight-forward.
20-03-2019

The proposed solution, following your proposal, was to introduce two boolean flags, like we did with DIsplayVMOutputToXX. Being boolean flags, these would be independent from each other switchable unless you manually do something about it, as it is done for DisplayVMOutputTo... in arguments.cpp, where those two flags are parsed manually to switch the other one off. Look, I can do it this way if you prefer, I have no preferences whatsoever. In my book, if you specify both flags you are using it wrong. I just would like to progress with this CSR. (Note that I briefly thought about having multiple destinations, e g printing to stdout AND stderr, but that opens another can of worms. Error reporting is usually a one-shot thing - too many things are broken - so you would have to store the report somewhere before sending it to multiple destinations. Which means allocating memory at crash time or preallocating it, both things I do not like).
20-03-2019

I don't follow. Arguments are parsed in order so each time we see a ErrorFileToA we disable ErrorFileToB and vice-versa. The end result is that the last option specified "wins".
20-03-2019

> I suggest that setting either flag disables the other (which can be stated in the globals.hpp text). That way "last one wins". Would make implementation more difficult since we loose argument position after parsing. Would be a clearly stated preference for stderr acceptable?
20-03-2019

Ok, I modified the text accordingly.
20-03-2019

> If both are specified, behavior is undefined. I suggest that setting either flag disables the other (which can be stated in the globals.hpp text). That way "last one wins".
19-03-2019

I am okay with -XX:+ErrorFileToStderr/Stdout instead of -XX:ErrorFileRedirect=(stdout|stderr). Only minor nit is that it makes not explicitly clear that they are mutually exclusive.
19-03-2019

Hi Thomas, Sorry the process is still not that clear from the CSR wiki or FAQs. It may be that the reviewer is only needed before being Finalized. For small hotspot flag changes we tend to go for the short review cycle: draft -> Finalized, which does require the reviewer in place. The "Specification" section should really only include a patch/webrev etc of the actual specification change, rather than a full implementation patch/webrev, to make it easier for reviewers and CSR group members to evaluate the actual change. For API changes this would just be a webrev or specdiff of the API javadoc. For hotspot flag changes it suffices to use simple text to describe it, or else the patch to globals.hpp that adds it. I found the email discussion after seeing the CSR. As I wrote there I think given we already have DisplayVMOutputToStderr/Stdout then something like -XX:+ErrorFileToStderr/Stdout may be the better choice.
18-03-2019

Hi David, I moved this back to "Draft". Seem to remember this differently - I thought only "proposed" CSRs are candidates for reviewing. I changed the Specification section as per your request. But please note that I filled this out according to the pre-filled CSR template, which specifies: <quote> Specification ------------- The detailed changes. Acceptable normative formats include inline patches, attached webrevs, and attached specdiffs. .. </quote> I used a gist in this case since Jira mauled the patch I copy-pasted in. But ok, I moved the patch into a webrev hosted at openjdk and posted a link to that instead. --- As for using ErrorFile - this was my first choice, but I actually prefer a separate switch for this because it minimized compatibility risk. Apart from someone naming his hs-err file "stdout" for some weird reason - which yes, I find unlikely too - someone may use "-XX:ErrorFile=stdout" on a JVM which is too old, not yet containing the proposed feature. Instead of getting an error on startup he would get a "stdout" file, only if the VM crashes. That is unexpected and may not be noticable before the JVM actually crashes.
18-03-2019

Hi [~stuefe], Please move this back to "draft". You need a Reviewer before you can propose this. The "Specification" section should be a clear description of the flag being added (basically what you have in the solution section), not a link to a webrev or implementation. Further any links should be to OpenJDK hosted environments not github - thanks. For new flags a copy of the proposed globals.hpp code suffices for the "specification". Can the existing ErrorFile flag not be used to achieve the same functionality somehow? I find it hard to believe anyone would currently deliberately use the names stdout or stderr.
18-03-2019