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

[liach]

On Sun, 19 Dec 2021 08:45:02 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> liach has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Just use volatile directly to ensure read order
>
> 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).

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

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