RFR: 8369469: Rdtsc: Remove potential races in Rdtsc::initialize

[David Holmes]

On Thu, 9 Oct 2025 06:18:14 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

> The whole chain of Rdtsc uses static block scope variables which are guaranteed to be constructed only once (implements some sort of double checked locking).
> However the current implementation does not do this correctly in all places, where it does:
> 
>   static bool initialized = false;
>   if (!initialized) {
>     ...
>     initialized = true;
>   }
> 
> It should do
> 
> static bool initialized = initialize_impl(); // Do the logic inside a function call
> 
> to guarantee that initialize is only called once.
> 
> We have observed this from some GC threads if we start using Ticks to early.
> 
> The suggested patch makes `Rdtsc::initialize` private an is only called from the `Rdtsc::enabled` which uses a static bock scoped variable to ensure the call once semantics. The property called from outside is now  `Rdtsc::enabled`. Which is more true to what how it is used on all call sites.

Seems reasonable.

One typo to fix.

Thanks

src/hotspot/cpu/x86/rdtsc_x86.cpp line 34:

> 32: #include "vm_version_x86.hpp"
> 33: 
> 34: DEBUG_ONLY(volatile int Rdtsc::_initalized = 0;)

Suggestion:

DEBUG_ONLY(volatile int Rdtsc::_initialized = 0;)

src/hotspot/cpu/x86/rdtsc_x86.cpp line 173:

> 171: 
> 172: bool Rdtsc::initialize() {
> 173:   precond(AtomicAccess::xchg(&_initalized, 1) == 0);

Suggestion:

  precond(AtomicAccess::xchg(&_initialized, 1) == 0);

src/hotspot/cpu/x86/rdtsc_x86.hpp line 42:

> 40: class Rdtsc : AllStatic {
> 41:  private:
> 42:   DEBUG_ONLY(static volatile int _initalized;)

Suggestion:

  DEBUG_ONLY(static volatile int _initialized;)

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

PR Review: https://git.openjdk.org/jdk/pull/27712#pullrequestreview-3329861518
PR Review Comment: https://git.openjdk.org/jdk/pull/27712#discussion_r2425134093
PR Review Comment: https://git.openjdk.org/jdk/pull/27712#discussion_r2425135899
PR Review Comment: https://git.openjdk.org/jdk/pull/27712#discussion_r2425136929