RFR: 8303422: Use a common functions to exit the VM for -Xshare:dump and CDS errors [v2]

[Ioi Lam]

On Thu, 13 Apr 2023 21:38:26 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> Currently, we have ad-hoc calls to os::_exit(0), vm_exit_during_initialization() and vm_direct_exit() in cases where there are dumptime or runtime errors for CDS. This patch introduces intuitively named methods for exiting the VM when there are dumptime/runtime errors and when dumping is finished. Verified with tier 1-4 tests.
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Coleen and Fred comments

Changes requested by iklam (Reviewer).

src/hotspot/share/cds/metaspaceShared.cpp line 270:

> 268:   _symbol_rs = ReservedSpace(symbol_rs_size);
> 269:   if (!_symbol_rs.is_reserved()) {
> 270:     MetaspaceShared::unrecoverable_writing_error(err_msg("Unable to reserve memory for symbols\n%ld bytes.", symbol_rs_size));

For complex message, it would be better to do this:


   log_error(cds)(Unable to reserve memory for symbols\n%ld bytes.", symbol_rs_siz);
   MetaspaceShared::unrecoverable_writing_error();


The `message` argument for `unrecoverable_writing_error()` should be used only for simple, fixed messages.

src/hotspot/share/cds/metaspaceShared.cpp line 569:

> 567:   // (such as vmClasses::_klasses) that may cause these VM operations
> 568:   // to fail. For safety, forget these operations and exit the VM directly.
> 569:   MetaspaceShared::exit_after_static_dump();

The above comment can be removed since we have a similar comment above the definition of exit_after_static_dump()

src/hotspot/share/cds/metaspaceShared.cpp line 912:

> 910:   jio_fprintf(defaultStream::error_stream(),
> 911:               "An error has occurred while processing the"
> 912:               " shared archive file.\n");

The above should use `log_error` to be consistent with the other CDS logging code. `log_error(cds)` goes to stdout by default. If we print part of the error message to stderr, that will make it hard to see all the error logs together.

(Some test cases may need to be adjusted accordingly).

src/hotspot/share/cds/metaspaceShared.cpp line 913:

> 911:               "An error has occurred while processing the"
> 912:               " shared archive file.\n");
> 913:   log_error(cds)("%s", message);

if message is null, then there's no need to print it.

src/hotspot/share/cds/metaspaceShared.cpp line 921:

> 919: void MetaspaceShared::unrecoverable_writing_error(const char* message) {
> 920:   tty->print_cr("%s", message);
> 921:   log_error(cds)("%s", message);

No need to write to tty since you're already writing to the log.

src/hotspot/share/cds/metaspaceShared.hpp line 119:

> 117:   static bool is_shared_dynamic(void* p) NOT_CDS_RETURN_(false);
> 118: 
> 119:   static void unrecoverable_loading_error(const char* message);

This can take a default argument `(const char* message = nullptr)`, so you don't need to pass "" when no extra message is needed.

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

PR Review: https://git.openjdk.org/jdk/pull/13463#pullrequestreview-1384464759
PR Review Comment: https://git.openjdk.org/jdk/pull/13463#discussion_r1166181277
PR Review Comment: https://git.openjdk.org/jdk/pull/13463#discussion_r1166181602
PR Review Comment: https://git.openjdk.org/jdk/pull/13463#discussion_r1166183052
PR Review Comment: https://git.openjdk.org/jdk/pull/13463#discussion_r1166179034
PR Review Comment: https://git.openjdk.org/jdk/pull/13463#discussion_r1166178676
PR Review Comment: https://git.openjdk.org/jdk/pull/13463#discussion_r1166177739