8219774: Reexamine the initialization of LangReflectAccess shared secret at AccessibleObject::<clinit>

Reviewed-by: alanb

src/hotspot/share/classfile/vmSymbols.hpp

@@ -247,9 +247,6 @@
   template(clazz_name,                                "clazz")                                    \
   template(exceptionTypes_name,                       "exceptionTypes")                           \
   template(modifiers_name,                            "modifiers")                                \
-  template(newConstructor_name,                       "newConstructor")                           \
-  template(newField_name,                             "newField")                                 \
-  template(newMethod_name,                            "newMethod")                                \
   template(invokeBasic_name,                          "invokeBasic")                              \
   template(linkToVirtual_name,                        "linkToVirtual")                            \
   template(linkToStatic_name,                         "linkToStatic")                             \

src/java.base/share/classes/java/lang/reflect/AccessibleObject.java

@@ -30,6 +30,7 @@ import java.lang.invoke.MethodHandle;
 import java.lang.ref.WeakReference;
 import java.security.AccessController;
 
+import jdk.internal.access.SharedSecrets;
 import jdk.internal.misc.VM;
 import jdk.internal.module.IllegalAccessLogger;
 import jdk.internal.reflect.CallerSensitive;
@@ -77,6 +78,10 @@ import sun.security.util.SecurityConstants;
  * @spec JPMS
  */
 public class AccessibleObject implements AnnotatedElement {
+    static {
+        // AccessibleObject is initialized early in initPhase1
+        SharedSecrets.setJavaLangReflectAccess(new ReflectAccess());
+    }
 
     static void checkPermission() {
         SecurityManager sm = System.getSecurityManager();

src/java.base/share/classes/java/lang/reflect/Constructor.java

@@ -111,7 +111,7 @@ public final class Constructor<T> extends Executable {
     /**
      * Package-private constructor used by ReflectAccess to enable
      * instantiation of these objects in Java code from the java.lang
-     * package via sun.reflect.LangReflectAccess.
+     * package via jdk.internal.access.JavaLangReflectAccess.
      */
     Constructor(Class<T> declaringClass,
                 Class<?>[] parameterTypes,

src/java.base/share/classes/java/lang/reflect/Field.java

@@ -113,9 +113,7 @@ class Field extends AccessibleObject implements Member {
 
 
     /**
-     * Package-private constructor used by ReflectAccess to enable
+     * Package-private constructor
-     * instantiation of these objects in Java code from the java.lang
-     * package via sun.reflect.LangReflectAccess.
      */
     Field(Class<?> declaringClass,
           String name,

src/java.base/share/classes/java/lang/reflect/Method.java

@@ -113,9 +113,7 @@ public final class Method extends Executable {
     }
 
     /**
-     * Package-private constructor used by ReflectAccess to enable
+     * Package-private constructor
-     * instantiation of these objects in Java code from the java.lang
-     * package via sun.reflect.LangReflectAccess.
      */
     Method(Class<?> declaringClass,
            String name,

src/java.base/share/classes/java/lang/reflect/Modifier.java

@@ -25,10 +25,7 @@
 
 package java.lang.reflect;
 
-import java.security.AccessController;
 import java.util.StringJoiner;
-import jdk.internal.reflect.LangReflectAccess;
-import jdk.internal.reflect.ReflectionFactory;
 
 /**
  * The Modifier class provides {@code static} methods and
@@ -47,16 +44,6 @@ import jdk.internal.reflect.ReflectionFactory;
  */
 public class Modifier {
 
-    /*
-     * Bootstrapping protocol between java.lang and java.lang.reflect
-     *  packages
-     */
-    static {
-        ReflectionFactory factory = AccessController.doPrivileged(
-                new ReflectionFactory.GetReflectionFactoryAction());
-        factory.setLangReflectAccess(new java.lang.reflect.ReflectAccess());
-    }
-
     /**
      * Return {@code true} if the integer argument includes the
      * {@code public} modifier, {@code false} otherwise.

src/java.base/share/classes/java/lang/reflect/ReflectAccess.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -29,52 +29,10 @@ import jdk.internal.reflect.MethodAccessor;
 import jdk.internal.reflect.ConstructorAccessor;
 
 /** Package-private class implementing the
-    sun.reflect.LangReflectAccess interface, allowing the java.lang
+    jdk.internal.access.JavaLangReflectAccess interface, allowing the java.lang
     package to instantiate objects in this package. */
 
-class ReflectAccess implements jdk.internal.reflect.LangReflectAccess {
+class ReflectAccess implements jdk.internal.access.JavaLangReflectAccess {
-    public Field newField(Class<?> declaringClass,
-                          String name,
-                          Class<?> type,
-                          int modifiers,
-                          int slot,
-                          String signature,
-                          byte[] annotations)
-    {
-        return new Field(declaringClass,
-                         name,
-                         type,
-                         modifiers,
-                         slot,
-                         signature,
-                         annotations);
-    }
-
-    public Method newMethod(Class<?> declaringClass,
-                            String name,
-                            Class<?>[] parameterTypes,
-                            Class<?> returnType,
-                            Class<?>[] checkedExceptions,
-                            int modifiers,
-                            int slot,
-                            String signature,
-                            byte[] annotations,
-                            byte[] parameterAnnotations,
-                            byte[] annotationDefault)
-    {
-        return new Method(declaringClass,
-                          name,
-                          parameterTypes,
-                          returnType,
-                          checkedExceptions,
-                          modifiers,
-                          slot,
-                          signature,
-                          annotations,
-                          parameterAnnotations,
-                          annotationDefault);
-    }
-
     public <T> Constructor<T> newConstructor(Class<T> declaringClass,
                                              Class<?>[] parameterTypes,
                                              Class<?>[] checkedExceptions,

src/java.base/share/classes/jdk/internal/access/JavaLangReflectAccess.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,38 +23,15 @@
  * questions.
  */
 
-package jdk.internal.reflect;
+package jdk.internal.access;
 
 import java.lang.reflect.*;
+import jdk.internal.reflect.*;
 
 /** An interface which gives privileged packages Java-level access to
     internals of java.lang.reflect. */
 
-public interface LangReflectAccess {
+public interface JavaLangReflectAccess {
-    /** Creates a new java.lang.reflect.Field. Access checks as per
-        java.lang.reflect.AccessibleObject are not overridden. */
-    public Field newField(Class<?> declaringClass,
-                          String name,
-                          Class<?> type,
-                          int modifiers,
-                          int slot,
-                          String signature,
-                          byte[] annotations);
-
-    /** Creates a new java.lang.reflect.Method. Access checks as per
-      java.lang.reflect.AccessibleObject are not overridden. */
-    public Method newMethod(Class<?> declaringClass,
-                            String name,
-                            Class<?>[] parameterTypes,
-                            Class<?> returnType,
-                            Class<?>[] checkedExceptions,
-                            int modifiers,
-                            int slot,
-                            String signature,
-                            byte[] annotations,
-                            byte[] parameterAnnotations,
-                            byte[] annotationDefault);
-
     /** Creates a new java.lang.reflect.Constructor. Access checks as
       per java.lang.reflect.AccessibleObject are not overridden. */
     public <T> Constructor<T> newConstructor(Class<T> declaringClass,

src/java.base/share/classes/jdk/internal/access/SharedSecrets.java

@@ -50,28 +50,29 @@ import jdk.internal.misc.Unsafe;
 
 public class SharedSecrets {
     private static final Unsafe unsafe = Unsafe.getUnsafe();
-    private static JavaUtilJarAccess javaUtilJarAccess;
+    private static JavaAWTAccess javaAWTAccess;
+    private static JavaAWTFontAccess javaAWTFontAccess;
+    private static JavaBeansAccess javaBeansAccess;
     private static JavaLangAccess javaLangAccess;
-    private static JavaLangModuleAccess javaLangModuleAccess;
     private static JavaLangInvokeAccess javaLangInvokeAccess;
+    private static JavaLangModuleAccess javaLangModuleAccess;
     private static JavaLangRefAccess javaLangRefAccess;
+    private static JavaLangReflectAccess javaLangReflectAccess;
     private static JavaIOAccess javaIOAccess;
+    private static JavaIOFileDescriptorAccess javaIOFileDescriptorAccess;
+    private static JavaIOFilePermissionAccess javaIOFilePermissionAccess;
+    private static JavaIORandomAccessFileAccess javaIORandomAccessFileAccess;
+    private static JavaObjectInputStreamAccess javaObjectInputStreamAccess;
+    private static JavaObjectInputFilterAccess javaObjectInputFilterAccess;
     private static JavaNetInetAddressAccess javaNetInetAddressAccess;
     private static JavaNetHttpCookieAccess javaNetHttpCookieAccess;
     private static JavaNetUriAccess javaNetUriAccess;
     private static JavaNetURLAccess javaNetURLAccess;
     private static JavaNioAccess javaNioAccess;
-    private static JavaIOFileDescriptorAccess javaIOFileDescriptorAccess;
+    private static JavaUtilJarAccess javaUtilJarAccess;
-    private static JavaIOFilePermissionAccess javaIOFilePermissionAccess;
-    private static JavaSecurityAccess javaSecurityAccess;
     private static JavaUtilZipFileAccess javaUtilZipFileAccess;
     private static JavaUtilResourceBundleAccess javaUtilResourceBundleAccess;
-    private static JavaAWTAccess javaAWTAccess;
+    private static JavaSecurityAccess javaSecurityAccess;
-    private static JavaAWTFontAccess javaAWTFontAccess;
-    private static JavaBeansAccess javaBeansAccess;
-    private static JavaObjectInputStreamAccess javaObjectInputStreamAccess;
-    private static JavaObjectInputFilterAccess javaObjectInputFilterAccess;
-    private static JavaIORandomAccessFileAccess javaIORandomAccessFileAccess;
     private static JavaSecuritySignatureAccess javaSecuritySignatureAccess;
     private static JavaxCryptoSealedObjectAccess javaxCryptoSealedObjectAccess;
 
@@ -129,6 +130,14 @@ public class SharedSecrets {
         return javaLangRefAccess;
     }
 
+    public static void setJavaLangReflectAccess(JavaLangReflectAccess jlra) {
+        javaLangReflectAccess = jlra;
+    }
+
+    public static JavaLangReflectAccess getJavaLangReflectAccess() {
+        return javaLangReflectAccess;
+    }
+
     public static void setJavaNetUriAccess(JavaNetUriAccess jnua) {
         javaNetUriAccess = jnua;
     }

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

@@ -43,6 +43,8 @@ import java.security.PrivilegedAction;
 import java.util.Objects;
 import java.util.Properties;
 
+import jdk.internal.access.JavaLangReflectAccess;
+import jdk.internal.access.SharedSecrets;
 import jdk.internal.misc.VM;
 import sun.reflect.misc.ReflectUtil;
 import sun.security.action.GetPropertyAction;
@@ -64,8 +66,7 @@ public class ReflectionFactory {
 
     private static boolean initted = false;
     private static final ReflectionFactory soleInstance = new ReflectionFactory();
-    // Provides access to package-private mechanisms in java.lang.reflect
+
-    private static volatile LangReflectAccess langReflectAccess;
 
     /* Method for static class initializer <clinit>, or null */
     private static volatile Method hasStaticInitializerMethod;
@@ -90,7 +91,9 @@ public class ReflectionFactory {
     // true if deserialization constructor checking is disabled
     private static boolean disableSerialConstructorChecks = false;
 
+    private final JavaLangReflectAccess langReflectAccess;
     private ReflectionFactory() {
+        this.langReflectAccess = SharedSecrets.getJavaLangReflectAccess();
     }
 
     /**
@@ -160,12 +163,7 @@ public class ReflectionFactory {
     //
     //
 
-    /** Called only by java.lang.reflect.Modifier's static initializer */
+    /*
-    public void setLangReflectAccess(LangReflectAccess access) {
-        langReflectAccess = access;
-    }
-
-    /**
      * Note: this routine can cause the declaring class for the field
      * be initialized and therefore must not be called until the
      * first get/set of this field.
@@ -175,7 +173,7 @@ public class ReflectionFactory {
     public FieldAccessor newFieldAccessor(Field field, boolean override) {
         checkInitted();
 
-        Field root = langReflectAccess().getRoot(field);
+        Field root = langReflectAccess.getRoot(field);
         if (root != null) {
             // FieldAccessor will use the root unless the modifiers have
             // been overrridden
@@ -197,7 +195,7 @@ public class ReflectionFactory {
         }
 
         // use the root Method that will not cache caller class
-        Method root = langReflectAccess().getRoot(method);
+        Method root = langReflectAccess.getRoot(method);
         if (root != null) {
             method = root;
         }
@@ -233,7 +231,7 @@ public class ReflectionFactory {
         }
 
         // use the root Constructor that will not cache caller class
-        Constructor<?> root = langReflectAccess().getRoot(c);
+        Constructor<?> root = langReflectAccess.getRoot(c);
         if (root != null) {
             c = root;
         }
@@ -268,52 +266,6 @@ public class ReflectionFactory {
     //
     //
 
-    /** Creates a new java.lang.reflect.Field. Access checks as per
-        java.lang.reflect.AccessibleObject are not overridden. */
-    public Field newField(Class<?> declaringClass,
-                          String name,
-                          Class<?> type,
-                          int modifiers,
-                          int slot,
-                          String signature,
-                          byte[] annotations)
-    {
-        return langReflectAccess().newField(declaringClass,
-                                            name,
-                                            type,
-                                            modifiers,
-                                            slot,
-                                            signature,
-                                            annotations);
-    }
-
-    /** Creates a new java.lang.reflect.Method. Access checks as per
-        java.lang.reflect.AccessibleObject are not overridden. */
-    public Method newMethod(Class<?> declaringClass,
-                            String name,
-                            Class<?>[] parameterTypes,
-                            Class<?> returnType,
-                            Class<?>[] checkedExceptions,
-                            int modifiers,
-                            int slot,
-                            String signature,
-                            byte[] annotations,
-                            byte[] parameterAnnotations,
-                            byte[] annotationDefault)
-    {
-        return langReflectAccess().newMethod(declaringClass,
-                                             name,
-                                             parameterTypes,
-                                             returnType,
-                                             checkedExceptions,
-                                             modifiers,
-                                             slot,
-                                             signature,
-                                             annotations,
-                                             parameterAnnotations,
-                                             annotationDefault);
-    }
-
     /** Creates a new java.lang.reflect.Constructor. Access checks as
         per java.lang.reflect.AccessibleObject are not overridden. */
     public Constructor<?> newConstructor(Class<?> declaringClass,
@@ -325,30 +277,20 @@ public class ReflectionFactory {
                                          byte[] annotations,
                                          byte[] parameterAnnotations)
     {
-        return langReflectAccess().newConstructor(declaringClass,
+        return langReflectAccess.newConstructor(declaringClass,
-                                                  parameterTypes,
+                                                parameterTypes,
-                                                  checkedExceptions,
+                                                checkedExceptions,
-                                                  modifiers,
+                                                modifiers,
-                                                  slot,
+                                                slot,
-                                                  signature,
+                                                signature,
-                                                  annotations,
+                                                annotations,
-                                                  parameterAnnotations);
+                                                parameterAnnotations);
-    }
-
-    /** Gets the MethodAccessor object for a java.lang.reflect.Method */
-    public MethodAccessor getMethodAccessor(Method m) {
-        return langReflectAccess().getMethodAccessor(m);
-    }
-
-    /** Sets the MethodAccessor object for a java.lang.reflect.Method */
-    public void setMethodAccessor(Method m, MethodAccessor accessor) {
-        langReflectAccess().setMethodAccessor(m, accessor);
     }
 
     /** Gets the ConstructorAccessor object for a
         java.lang.reflect.Constructor */
     public ConstructorAccessor getConstructorAccessor(Constructor<?> c) {
-        return langReflectAccess().getConstructorAccessor(c);
+        return langReflectAccess.getConstructorAccessor(c);
     }
 
     /** Sets the ConstructorAccessor object for a
@@ -356,21 +298,21 @@ public class ReflectionFactory {
     public void setConstructorAccessor(Constructor<?> c,
                                        ConstructorAccessor accessor)
     {
-        langReflectAccess().setConstructorAccessor(c, accessor);
+        langReflectAccess.setConstructorAccessor(c, accessor);
     }
 
     /** Makes a copy of the passed method. The returned method is a
         "child" of the passed one; see the comments in Method.java for
         details. */
     public Method copyMethod(Method arg) {
-        return langReflectAccess().copyMethod(arg);
+        return langReflectAccess.copyMethod(arg);
     }
 
     /** Makes a copy of the passed method. The returned method is NOT
      * a "child" but a "sibling" of the Method in arg. Should only be
      * used on non-root methods. */
     public Method leafCopyMethod(Method arg) {
-        return langReflectAccess().leafCopyMethod(arg);
+        return langReflectAccess.leafCopyMethod(arg);
     }
 
 
@@ -378,30 +320,30 @@ public class ReflectionFactory {
         "child" of the passed one; see the comments in Field.java for
         details. */
     public Field copyField(Field arg) {
-        return langReflectAccess().copyField(arg);
+        return langReflectAccess.copyField(arg);
     }
 
     /** Makes a copy of the passed constructor. The returned
         constructor is a "child" of the passed one; see the comments
         in Constructor.java for details. */
     public <T> Constructor<T> copyConstructor(Constructor<T> arg) {
-        return langReflectAccess().copyConstructor(arg);
+        return langReflectAccess.copyConstructor(arg);
     }
 
     /** Gets the byte[] that encodes TypeAnnotations on an executable.
      */
     public byte[] getExecutableTypeAnnotationBytes(Executable ex) {
-        return langReflectAccess().getExecutableTypeAnnotationBytes(ex);
+        return langReflectAccess.getExecutableTypeAnnotationBytes(ex);
     }
 
     public Class<?>[] getExecutableSharedParameterTypes(Executable ex) {
-        return langReflectAccess().getExecutableSharedParameterTypes(ex);
+        return langReflectAccess.getExecutableSharedParameterTypes(ex);
     }
 
     public <T> T newInstance(Constructor<T> ctor, Object[] args, Class<?> caller)
         throws IllegalAccessException, InstantiationException, InvocationTargetException
     {
-        return langReflectAccess().newInstance(ctor, args, caller);
+        return langReflectAccess.newInstance(ctor, args, caller);
     }
 
     //--------------------------------------------------------------------------
@@ -526,13 +468,13 @@ public class ReflectionFactory {
                                           constructorToCall.getParameterTypes(),
                                           constructorToCall.getExceptionTypes(),
                                           constructorToCall.getModifiers(),
-                                          langReflectAccess().
+                                          langReflectAccess.
                                           getConstructorSlot(constructorToCall),
-                                          langReflectAccess().
+                                          langReflectAccess.
                                           getConstructorSignature(constructorToCall),
-                                          langReflectAccess().
+                                          langReflectAccess.
                                           getConstructorAnnotations(constructorToCall),
-                                          langReflectAccess().
+                                          langReflectAccess.
                                           getConstructorParameterAnnotations(constructorToCall));
         setConstructorAccessor(c, acc);
         c.setAccessible(true);
@@ -725,17 +667,6 @@ public class ReflectionFactory {
         initted = true;
     }
 
-    private static LangReflectAccess langReflectAccess() {
-        if (langReflectAccess == null) {
-            // Call a static method to get class java.lang.reflect.Modifier
-            // initialized. Its static initializer will cause
-            // setLangReflectAccess() to be called from the context of the
-            // java.lang.reflect package.
-            Modifier.isPublic(Modifier.PUBLIC);
-        }
-        return langReflectAccess;
-    }
-
     /**
      * Returns true if classes are defined in the classloader and same package, false
      * otherwise.