RFR: 8354954: Typed static memory for late initialization of static class members in Hotspot

[Jorn Vernee]

On Wed, 16 Apr 2025 12:36:45 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Hi,
> 
> This PR introduces a `StableValue<T>` which is sized and aligned identically to a `T`, with the difference that a `StableValue<T>` needs to be explicitly instantiated.
> 
> Dynamic static initalization in C++ leads to unpredictable bugs as there is no defined order in which objects will be initialized, to the degree that 'static initialization fiasco' is a term used. In the code I've worked on in Hotspot we resolve this by having an initialization function, and instead of having static members of `T` we have `T*` instead and use `malloc` in order to gain the memory for the objects.
> 
> This is fine, however when working with certain code you may want to allocate before NMT is initialized. This leads to you having to use raw malloc. This is probably something you find out after having used `os::malloc` and already had a strange crash when building your code.
> 
> That's why I'd like to have `StableValue<T>`. It let's you avoid the whole `malloc` thing, and we overload `->` to make it behave as if it is actually a `T`. We add in a simple checker in debug mode that checks whether the memory has been initialized before using it.
> 
> In the code I've switched two members to be of `StableValue` instead. One is the malloc case above, the second (MemBaseline) is one where I got a bug while developing. The bug occurred because I  changed the initializer of `MemBaseline` without knowing that it was dynamic-static-allocated, and the exact change I made caused weird crashes (because of initialization order issues).
> 
> This solution is quite practical to me, but I wanted to know what others think.

Note that strict aliasing is turned off for hotspot (see `make/autoconf/flags-cflags.m4`). That said, the latest proposed version looks very good (with the suggestion to change the impl of `as()` to `return &_t;`).

src/hotspot/share/nmt/memoryFileTracker.cpp line 129:

> 127: bool MemoryFileTracker::Instance::initialize(NMT_TrackingLevel tracking_level) {
> 128:   if (tracking_level == NMT_TrackingLevel::NMT_off) return true;
> 129:   new (_tracker.as()) MemoryFileTracker(tracking_level == NMT_TrackingLevel::NMT_detail);

Maybe you could add an `init` function that forwards the constructor arguments, with an extra check to see if the memory has already been initialized:


  template<typename...As>
  void init(As&&...args) {
    assert(is_death_pattern(), "StaticArea already initialized");
    new (as()) T(std::forward<As>(args)...);
  }


Suggestion:

  _tracker.init(tracking_level == NMT_TrackingLevel::NMT_detail);

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

PR Comment: https://git.openjdk.org/jdk/pull/24689#issuecomment-2812731591
PR Review Comment: https://git.openjdk.org/jdk/pull/24689#discussion_r2047054839