RFR: 8271003: hs_err improvement: handle CLASSPATH env setting longer than O_BUFLEN

[Thomas Stuefe]

On Fri, 30 Jul 2021 18:17:54 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

> Please review this small enhancement for addressing the problem of the CLASSPATH env variable setting being truncated in a hs err log.
> 
> For printing a char string, it doesn't need to go through `do_vsnprintf()` which does the truncation based on the input buffer length. The change is local to the code path pertaining to hs err log.
> 
> Testing:
> 
> - [x] tiers 1, 2 (including the new test)

Hi Calvin,

Sorry, I think your patch is not a good solution.

The problem is that the print_cr() functions are supposed to write the newline *with the same `write(2)` call*. That is not documented in do_vsnprintf(), but maybe we should do that since its an easy to miss a detail.

Writing with one single `write(2)` call guarantees some atomicity when writing concurrently from several threads. If you write the newline separately, you risk line tear - the newline appearing away from its line.

This means that `print_cr("%s", s)` does not benefit from your improvement. Only `print("%s", s)` would, but here, we do already handle the two special cases in exactly the way you would.

I propose a specific fix for your problem in `os::print_environment_variables()` by replacing `st->print_cr("%s", envvar);` with either one of

st->print("%s", envvar);
st->cr()

or

st->print_raw(envvar);
st->cr()


The test is useful though. But you can simplify it: 

- as David said, use `-XX:ErrorHandlerTest`. For better realistic testing, I'd use an option which gives you a real signal, e.g. `-XX:ErrorHandlerTest=14`
- just start the test with `-XX:+ErrorFileToStdout`. We added this option to pipe the error file to stdout. That way you can parse stdout directly, using OutputScanner methods and regex, no need to open and parse the hs-err file:


thomas at starfish$ CLASSPATH=abcabc ./images/jdk/bin/java -XX:ErrorHandlerTest=14 -XX:+ErrorFileToStdout | grep CLASSPATH
CLASSPATH=abcabc


Cheers, Thomas

-------------

Changes requested by stuefe (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4947