[8u] RFR: JDK-8220786: Create new switch to redirect error reporting output to stdout or stderr

[Andrew Hughes]

On 21:36 Tue 01 Jun     , Volker Simonis wrote:
> Hi,
> 
> can I please get a review for this backport?
> 
> This change initially went into jdk 13 and has already been downported
> to jdk 11u more than a year ago. But as jdk8u is still used a lot and
> increasingly in containerized environments I think it still makes a
> lot of sense to downport it to jdk 8u as well.
> 
> If the JVM crashes in a containerized environment with its ephemeral
> file system it becomes impossible to do any post mortem analysis of
> the problem. In such situations the ability to dump the hs_err file to
> stdout/stderr is very useful, because many container environments log
> the stdout/stderr stream by default.
> 
> Original issue: https://bugs.openjdk.java.net/browse/JDK-8220786
> Original CSR: https://bugs.openjdk.java.net/browse/JDK-8220787
> Original change: http://hg.openjdk.java.net/jdk/jdk/rev/cf75ea6af695
> 
> jdk11 backport: https://bugs.openjdk.java.net/browse/JDK-8238531
> jdk11 CSR: https://bugs.openjdk.java.net/browse/JDK-8238532
> jdk11 change: https://hg.openjdk.java.net/jdk-updates/jdk11u/rev/783a56f3ac1f
> 
> jdk8 backport: https://bugs.openjdk.java.net/browse/JDK-8268055
> jdk8 CSR: https://bugs.openjdk.java.net/browse/JDK-8268058
> jdk8 webrev: http://cr.openjdk.java.net/~simonis/webrevs/2021/8220786-jdk8u/
> 
> I had to do quite some, but trivial changes to the jdk 11 backport
> (which I started with) to fit it into jdk 8u:
> 
> arguments.cpp
> -------------------
> In jdk 11 "FLAG_SET_CMDLINE(boolean, ...)" expands to "JVMFlag::Error
> boolAtPut(JVMFlagsWithType flag, bool value, JVMFlag::Flags origin)"
> which returns a "JVMFlag::Error" and can be checked for success. In
> jdk 8,  "FLAG_SET_CMDLINE(boolean, ...)" expands to "void
> boolAtPut(CommandLineFlagWithType flag, bool value, Flag::Flags
> origin)" which returns void, so there's nothing to check for...
> 
> vmError.cpp
> ----------------
> I think the initial implementation has a bug in the following code:
> 
> -    if (fd_log != -1) {
> +    if (fd_log > 3) {
>        close(fd_log);
> 
> I think the intention is to close the log stream if it was not stdout
> (i.e. "1") or stderr (i.e. "2") but it checks for "fd_log > 3".
> 
> I've fixed this in the downport but I'm happy to revert the fix if
> somebody has a good explanation for the original code:
> 
> -    if (log.fd() != defaultStream::output_fd()) {
> +    if (log.fd() > 2) {
> 
> ErrorFileRedirectTest.java
> ---------------------------------
> - path/package shuffling for the test library
> - use "Platform.isDebugBuild()" instead of "@requires (vm.debug ==
> true)" which is not supported in jdk8
> - remove "-XX:-CreateCoredumpOnCrash" option because it doesn't exist in jdk 8
> - use "-XX:ErrorHandlerTest=12" instead of "-XX:ErrorHandlerTest=14"
> because "14" is not available in jdk 8. But "12" still generates a
> SIGSEV by reading from memory address 0.
> - check for "T H R E A D" in the output because jdk 8 has no "S U M M
> A R Y" section.
> 
> The jdk 8 CSR (https://bugs.openjdk.java.net/browse/JDK-8268058) has
> been reviewed and waits for approval.
> 
> Thank you and best regards,
> Volker
> 

Thanks for the comprehensive log of the changes needed for 8u.

I assume you rolled back the local change to > 2 as Paul suggested, as I
don't see it in:

https://cr.openjdk.java.net/~simonis/webrevs/2021/8220786-jdk8u/hotspot.changeset

That seems the right thing to me. This should be handled under its own bug.

That changeset can be considered reviewed and approved.

Thanks,
-- 
Andrew :)
Pronouns: he / him or they / them
Senior Free Java Software Engineer
OpenJDK Package Owner
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222