8261407: ReflectionFactory.checkInitted() is not thread-safe

Co-authored-by: Peter Levart <[email protected]>
Reviewed-by: dholmes, mchung, plevart

Author: 2 people

src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java

@@ -44,6 +44,7 @@
 import jdk.internal.access.JavaLangReflectAccess;
 import jdk.internal.access.SharedSecrets;
 import jdk.internal.misc.VM;
+import jdk.internal.vm.annotation.Stable;
 import sun.security.action.GetPropertyAction;
 import sun.security.util.SecurityConstants;
 
@@ -61,42 +62,12 @@
 
 public class ReflectionFactory {
 
-    private static boolean initted = false;
     private static final ReflectionFactory soleInstance = new ReflectionFactory();
 
 
     /* Method for static class initializer <clinit>, or null */
     private static volatile Method hasStaticInitializerMethod;
 
-    //
-    // "Inflation" mechanism. Loading bytecodes to implement
-    // Method.invoke() and Constructor.newInstance() currently costs
-    // 3-4x more than an invocation via native code for the first
-    // invocation (though subsequent invocations have been benchmarked
-    // to be over 20x faster). Unfortunately this cost increases
-    // startup time for certain applications that use reflection
-    // intensively (but only once per class) to bootstrap themselves.
-    // To avoid this penalty we reuse the existing JVM entry points
-    // for the first few invocations of Methods and Constructors and
-    // then switch to the bytecode-based implementations.
-    //
-    // Package-private to be accessible to NativeMethodAccessorImpl
-    // and NativeConstructorAccessorImpl
-    private static boolean noInflation        = false;
-    private static int     inflationThreshold = 15;
-
-    //
-    // New implementation uses direct invocation of method handles
-    private static final int METHOD_MH_ACCESSOR      = 0x1;
-    private static final int FIELD_MH_ACCESSOR       = 0x2;
-    private static final int ALL_MH_ACCESSORS        = METHOD_MH_ACCESSOR|FIELD_MH_ACCESSOR;
-
-    private static int     useDirectMethodHandle = ALL_MH_ACCESSORS;
-    private static boolean useNativeAccessorOnly = false;  // for testing only
-
-    // true if deserialization constructor checking is disabled
-    private static boolean disableSerialConstructorChecks = false;
-
     private final JavaLangReflectAccess langReflectAccess;
     private ReflectionFactory() {
         this.langReflectAccess = SharedSecrets.getJavaLangReflectAccess();
@@ -160,8 +131,6 @@ public static ReflectionFactory getReflectionFactory() {
      * @param override true if caller has overridden accessibility
      */
     public FieldAccessor newFieldAccessor(Field field, boolean override) {
-        checkInitted();
-
         Field root = langReflectAccess.getRoot(field);
         if (root != null) {
             // FieldAccessor will use the root unless the modifiers have
@@ -180,8 +149,6 @@ public FieldAccessor newFieldAccessor(Field field, boolean override) {
     }
 
     public MethodAccessor newMethodAccessor(Method method, boolean callerSensitive) {
-        checkInitted();
-
         // use the root Method that will not cache caller class
         Method root = langReflectAccess.getRoot(method);
         if (root != null) {
@@ -191,7 +158,7 @@ public MethodAccessor newMethodAccessor(Method method, boolean callerSensitive)
         if (useMethodHandleAccessor()) {
             return MethodHandleAccessorFactory.newMethodAccessor(method, callerSensitive);
         } else {
-            if (noInflation && !method.getDeclaringClass().isHidden()) {
+            if (noInflation() && !method.getDeclaringClass().isHidden()) {
                 return generateMethodAccessor(method);
             } else {
                 NativeMethodAccessorImpl acc = new NativeMethodAccessorImpl(method);
@@ -215,8 +182,6 @@ static MethodAccessorImpl generateMethodAccessor(Method method) {
     }
 
     public ConstructorAccessor newConstructorAccessor(Constructor<?> c) {
-        checkInitted();
-
         Class<?> declaringClass = c.getDeclaringClass();
         if (Modifier.isAbstract(declaringClass.getModifiers())) {
             return new InstantiationExceptionConstructorAccessorImpl(null);
@@ -242,7 +207,7 @@ public ConstructorAccessor newConstructorAccessor(Constructor<?> c) {
                 return new BootstrapConstructorAccessorImpl(c);
             }
 
-            if (noInflation && !c.getDeclaringClass().isHidden()) {
+            if (noInflation() && !c.getDeclaringClass().isHidden()) {
                 return new MethodAccessorGenerator().
                         generateConstructor(c.getDeclaringClass(),
                                             c.getParameterTypes(),
@@ -430,7 +395,7 @@ public final Constructor<?> newConstructorForSerialization(Class<?> cl) {
         while (Serializable.class.isAssignableFrom(initCl)) {
             Class<?> prev = initCl;
             if ((initCl = initCl.getSuperclass()) == null ||
-                (!disableSerialConstructorChecks && !superHasAccessibleConstructor(prev))) {
+                (!disableSerialConstructorChecks() && !superHasAccessibleConstructor(prev))) {
                 return null;
             }
         }
@@ -623,41 +588,108 @@ public final Constructor<OptionalDataException> newOptionalDataExceptionForSeria
     // Internals only below this point
     //
 
+    // Package-private to be accessible to NativeMethodAccessorImpl
+    // and NativeConstructorAccessorImpl
     static int inflationThreshold() {
-        return inflationThreshold;
+        return config().inflationThreshold;
     }
 
     static boolean noInflation() {
-        return noInflation;
+        return config().noInflation;
     }
 
     static boolean useMethodHandleAccessor() {
-        return (useDirectMethodHandle & METHOD_MH_ACCESSOR) == METHOD_MH_ACCESSOR;
+        return (config().useDirectMethodHandle & METHOD_MH_ACCESSOR) == METHOD_MH_ACCESSOR;
     }
 
     static boolean useFieldHandleAccessor() {
-        return (useDirectMethodHandle & FIELD_MH_ACCESSOR) == FIELD_MH_ACCESSOR;
+        return (config().useDirectMethodHandle & FIELD_MH_ACCESSOR) == FIELD_MH_ACCESSOR;
     }
 
     static boolean useNativeAccessorOnly() {
-        return useNativeAccessorOnly;
+        return config().useNativeAccessorOnly;
     }
 
-    /** We have to defer full initialization of this class until after
-        the static initializer is run since java.lang.reflect.Method's
-        static initializer (more properly, that for
-        java.lang.reflect.AccessibleObject) causes this class's to be
-        run, before the system properties are set up. */
-    private static void checkInitted() {
-        if (initted) return;
+    private static boolean disableSerialConstructorChecks() {
+        return config().disableSerialConstructorChecks;
+    }
+
+    // New implementation uses direct invocation of method handles
+    private static final int METHOD_MH_ACCESSOR = 0x1;
+    private static final int FIELD_MH_ACCESSOR = 0x2;
+    private static final int ALL_MH_ACCESSORS = METHOD_MH_ACCESSOR | FIELD_MH_ACCESSOR;
+
+    /**
+     * The configuration is lazily initialized after the module system is initialized. The
+     * default config would be used before the proper config is loaded.
+     *
+     * The static initializer of ReflectionFactory is run before the system properties are set up.
+     * The class initialization is caused by the class initialization of java.lang.reflect.Method
+     * (more properly, caused by the class initialization for java.lang.reflect.AccessibleObject)
+     * that happens very early VM startup, initPhase1.
+     */
+    private static @Stable Config config;
+
+    // "Inflation" mechanism. Loading bytecodes to implement
+    // Method.invoke() and Constructor.newInstance() currently costs
+    // 3-4x more than an invocation via native code for the first
+    // invocation (though subsequent invocations have been benchmarked
+    // to be over 20x faster). Unfortunately this cost increases
+    // startup time for certain applications that use reflection
+    // intensively (but only once per class) to bootstrap themselves.
+    // To avoid this penalty we reuse the existing JVM entry points
+    // for the first few invocations of Methods and Constructors and
+    // then switch to the bytecode-based implementations.
+
+    private static final Config DEFAULT_CONFIG = new Config(false, // noInflation
+                                                            15, // inflationThreshold
+                                                            ALL_MH_ACCESSORS, // useDirectMethodHandle
+                                                            false, // useNativeAccessorOnly
+                                                            false); // disableSerialConstructorChecks
+
+    /**
+     * The configurations for the reflection factory. Configurable via
+     * system properties but only available after ReflectionFactory is
+     * loaded during early VM startup.
+     *
+     * Note that the default implementations of the object methods of
+     * this Config record (toString, equals, hashCode) use indy,
+     * which is available to use only after initPhase1. These methods
+     * are currently not called, but should they be needed, a workaround
+     * is to override them.
+     */
+    private record Config(boolean noInflation,
+                          int inflationThreshold,
+                          int useDirectMethodHandle,
+                          boolean useNativeAccessorOnly,
+                          boolean disableSerialConstructorChecks) {
+    }
+
+    private static Config config() {
+        Config c = config;
+        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;
+            return DEFAULT_CONFIG;
         }
 
+        return config = loadConfig();
+    }
+
+    private static Config loadConfig() {
+        assert VM.isModuleSystemInited();
+
+        boolean noInflation = DEFAULT_CONFIG.noInflation;
+        int inflationThreshold = DEFAULT_CONFIG.inflationThreshold;
+        int useDirectMethodHandle = DEFAULT_CONFIG.useDirectMethodHandle;
+        boolean useNativeAccessorOnly = DEFAULT_CONFIG.useNativeAccessorOnly;
+        boolean disableSerialConstructorChecks = DEFAULT_CONFIG.disableSerialConstructorChecks;
+
         Properties props = GetPropertyAction.privilegedGetProperties();
         String val = props.getProperty("sun.reflect.noInflation");
         if (val != null && val.equals("true")) {
@@ -690,7 +722,11 @@ private static void checkInitted() {
         disableSerialConstructorChecks =
             "true".equals(props.getProperty("jdk.disableSerialConstructorChecks"));
 
-        initted = true;
+        return new Config(noInflation,
+                          inflationThreshold,
+                          useDirectMethodHandle,
+                          useNativeAccessorOnly,
+                          disableSerialConstructorChecks);
     }
 
     /**