RFR(s): 8214975: No hs-err file if fatal error is raised during dynamic initialization.

[Daniel D. Daugherty]

On 12/8/18 6:21 AM, Thomas Stüfe wrote:
> Hi,
>
> may I please have reviews for this small patch:
>
> https://bugs.openjdk.java.net/browse/JDK-8214975
> http://cr.openjdk.java.net/~stuefe/webrevs/8214975-no-hs-errfile-on-early-assert/webrev.00/webrev

src/hotspot/share/utilities/debug.cpp
     No comments.

src/hotspot/share/utilities/vmError.cpp
     No comments.

src/hotspot/share/utilities/vmError.hpp
     No comments.
test/hotspot/jtreg/runtime/ErrorHandling/VeryEarlyAssertTest.java
     Nice test. Thanks!

Thumbs up!

Dan


>
> A fatal error (e.g. assert), if triggered during dynamic
> initialization, will plain kill the VM without a trace. (A)
>
> A alternative variant of this error: VM will get caught up in an
> endless recursion, repeat "[Too many errors, abort]\n" endlessly,
> quickly growing its RSS until the OOM Killer kills it. (B)
>
> These symptoms are all caused by VMError::report() attempting to write
> to an uninitialized fdStream object (VMError::out and VMError::log).
> These object instances are allocated at global file scope:
>
> 1199 fdStream VMError::out(defaultStream::output_fd());
> 1200 fdStream VMError::log; // error log used by VMError::report_and_die()
>
> They are non-trivial (have vtables) and need to be initialized
> themselves before being used. If the assert happens before they are
> initialized, the vtables will not yet have been set up, and once we
> attempt to call out::write() or log::write(), in an context where we
> only have an outputStream* ptr (e.g. VMError::report()), we crash.
>
> Depending on which one of VMError::log and VMError::out are still
> uninitialized, we end up in (A) or (B). In both cases, the secondary
> signal handler (crash_handler()) will catch the signal, re-try error
> reporting and crash again. This is an endless cycle. We do have
> recursion detection in place to stop this cycle:
>
> 1388 if (recursive_error_count++ > 30) {
> 1389 out.print_raw_cr("[Too many errors, abort]");
> 1390 os::die();
> 1391 }
>
> but this again uses methods of outputStream to write the "Too many
> errors" text, which will crash again before ever reaching os::die().
>
> --
>
> Solution: do not rely on global non-trivial C++ objects, especially
> not at global scope. Instead, keep the information which must be
> preserved across recursions and different threads (which are only the
> logfile handles themselves) in non-c++ (POD) ints.
>
> As added safety, do not keep them at global scope but at static
> function-scope inside VMError::report_and_die() - that moves their
> initialization back to the first time they are called. To print, we
> still need outputStream objects of course, but they can be restricted
> to function scope inside VMError::report_and_die(), and can be
> recreated each time this function is entered, be it because of
> recursions of from a different thread.
>
> Additionally, a jtreg regression test is added.
>
> Thanks, Thomas