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

[Peter Levart]

On Thu, 10 Feb 2022 22:53:56 GMT, liach <duke at openjdk.java.net> wrote:

>> Upon review of [8261407](https://bugs.openjdk.java.net/browse/JDK-8261407), by design, duplicate initialization of ReflectionFactory should be safe as it performs side-effect-free property read actions, and the suggesting of making the `initted` field volatile cannot prevent concurrent initialization either; however, having `initted == true` published without the other fields' values is a possibility, which this patch addresses.
>> 
>> This simulates what's done in `CallSite`'s constructor for `ConstantCallSite`. Please feel free to point out the problems with this patch, as I am relatively inexperienced in this field of fences and there are relatively less available documents. (Thanks to https://shipilev.net/blog/2014/on-the-fence-with-dependencies/)
>
> liach has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Make config a pojo, move loading code into config class

Changes requested by plevart (Reviewer).

src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 685:

> 683:                 instance = c = load();
> 684:             }
> 685:             return c;

If you do that the "old" way, you loose the ability for JIT to constant-fold the `instance` and by transitivity the Config instance fields, since the check for `VM.isModuleSystemInited()` can't be elided. As suggested, the fast-path check should be done 1st, like:


    private static @Stable Config instance;

    private static Config instance() {
        Config c = instance;
        if (c != null) {
            return c;
        }
        
        // Defer initialization until module system is initialized so as
        // to avoid inflation and spinning bytecode in unnamed modules
        // during early startup.
        if (!VM.isModuleSystemInited()) {
            return DEFAULT;
        }

        instance = c = load();
        return c;
    }

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

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