RFR: 8340420: ZGC: Should call `vm_shutdown_during_initialization` if initialization fails

[Stefan Karlsson]

On Mon, 30 Sep 2024 06:36:21 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

> ZGC does not call `vm_shutdown_during_initialization` if initialization fails during the setup of the CollectedHeap, in contrast to the other GC.
> 
> I propose we add a `ZInitialize::error` which we can use during initialisation to record errors. The first error recored is also stored and used as the error message when shutting down the VM. 
> 
> Initially used malloc to allocate the error (ed9ba5dd6805291a6b1b56566c933424230d3b4a) but feels like it is just better to have static storage for the string and not have to care about malloc potentially failing to allocate.

Changes requested by stefank (Reviewer).

src/hotspot/share/gc/z/zInitialize.cpp line 95:

> 93:   va_list argp;
> 94:   va_start(argp, msg_format);
> 95:   const FormatBuffer<ErrorMessageLength> error(FormatBufferDummy(), msg_format, argp);

Maybe consider a name that doesn't clash with the function name.

src/hotspot/share/gc/z/zInitialize.cpp line 103:

> 101:   va_list argp;
> 102:   va_start(argp, msg_format);
> 103:   const FormatBuffer<ErrorMessageLength> error(FormatBufferDummy(), msg_format, argp);

Maybe consider a name that doesn't clash with the function name.

src/hotspot/share/gc/z/zInitialize.cpp line 117:

> 115:     return "Unknown error, check error GC logs";
> 116:   }
> 117:   return _error_message;

Maybe invert the conditional?
Suggestion:

  if (had_error()) {
    return _error_message;
  }
  return "Unknown error, check error GC logs";

src/hotspot/share/gc/z/zInitialize.hpp line 35:

> 33: 
> 34: class ZInitializer {
> 35:  public:

Indentation doesn't follow the ZGC style.

src/hotspot/share/gc/z/zInitialize.hpp line 42:

> 40: private:
> 41:   static constexpr size_t ErrorMessageLength = 256;
> 42:   static char _error_message[ErrorMessageLength];

I would suggest that we separate the constants
Suggestion:

  static constexpr size_t ErrorMessageLength = 256;

  static char _error_message[ErrorMessageLength];

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

PR Review: https://git.openjdk.org/jdk/pull/21254#pullrequestreview-2336623596
PR Review Comment: https://git.openjdk.org/jdk/pull/21254#discussion_r1780576948
PR Review Comment: https://git.openjdk.org/jdk/pull/21254#discussion_r1780576635
PR Review Comment: https://git.openjdk.org/jdk/pull/21254#discussion_r1780577740
PR Review Comment: https://git.openjdk.org/jdk/pull/21254#discussion_r1780578369
PR Review Comment: https://git.openjdk.org/jdk/pull/21254#discussion_r1780579084