RFR: 8261407: ReflectionFactory.checkInitted() is not thread-safe [v2]

[Alan Bateman]

On Sun, 19 Dec 2021 20:34:22 GMT, liach <duke at openjdk.java.net> wrote:

>> src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 65:
>> 
>>> 63: 
>>> 64:     // volatile ensures static fields set before are published
>>> 65:     private static volatile boolean initted = false;
>> 
>> I assume you can avoid the volatile write by not explicitly initialising it to false. The other thing to try here is making it a @Stable field.
>> 
>> There are issues with noInflation inflationThreshold too but since they are no longer used by default then I think we can leave them, that code will eventually be removed.
>
> Hmm, one of my original draft was like:
> 
>     @Stable
>     private static boolean initted;
> 
> 
>         // ensure previous fields are visible before initted is
>         var unsafe = Unsafe.getUnsafe();
>         unsafe.storeStoreFence();
>         initted = true;
>         unsafe.fullFence(); // Serves as the store-load fence
>         // May this simply be a store-store fence like in ConstantCallSite?
> 
> The store-load fence is described by [shipilev's article](https://shipilev.net/blog/2014/on-the-fence-with-dependencies/#_model) as inserted after volatile writes.
> 
> I doubt if simply marking it `@Stable` can make it safe: in [`ConstantCallSite`'s constructor](https://github.com/openjdk/jdk/blob/63e43030ed1260d14df950342c39a377231a3f40/src/java.base/share/classes/java/lang/invoke/ConstantCallSite.java#L52), a store-store fence is used to publish a stable field (note the super constructor calls a store-store fence at its end as well).

> Hmm, one of my original draft was like:
> 
> ```java
>     @Stable
>     private static boolean initted;
> ```

The suggestion was to try `@Stable private static volatile boolean inited` to see if the volatile read is elided from the generated code. It does can be looked at later too as what you have is okay (assuming the explicitly initialisation to false is removed).

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

PR: https://git.openjdk.java.net/jdk/pull/6889