changeset 12689:9b7793714406

Remove Reflection.quickCheckMemberAccess Beef-up jlr.Module javadoc
author alanb
date Tue, 12 May 2015 11:40:05 +0100
parents e8e91fa7c4e3
children 908730244109
files src/java.base/share/classes/java/lang/Class.java src/java.base/share/classes/java/lang/reflect/Constructor.java src/java.base/share/classes/java/lang/reflect/Field.java src/java.base/share/classes/java/lang/reflect/Method.java src/java.base/share/classes/java/lang/reflect/Module.java src/java.base/share/classes/sun/reflect/Reflection.java test/jdk/jigsaw/reflect/readable/src/m1/one/Main.java
diffstat 7 files changed, 107 insertions(+), 128 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/lang/Class.java	Tue May 12 07:55:15 2015 +0100
+++ b/src/java.base/share/classes/java/lang/Class.java	Tue May 12 11:40:05 2015 +0100
@@ -522,13 +522,11 @@
         }
         Constructor<T> tmpConstructor = cachedConstructor;
         // Security check (same as in java.lang.reflect.Constructor)
-        int modifiers = tmpConstructor.getModifiers();
-        if (!Reflection.quickCheckMemberAccess(this, modifiers)) {
-            Class<?> caller = Reflection.getCallerClass();
-            if (newInstanceCallerCache != caller) {
-                Reflection.ensureMemberAccess(caller, this, null, modifiers);
-                newInstanceCallerCache = caller;
-            }
+        Class<?> caller = Reflection.getCallerClass();
+        if (newInstanceCallerCache != caller) {
+            int modifiers = tmpConstructor.getModifiers();
+            Reflection.ensureMemberAccess(caller, this, null, modifiers);
+            newInstanceCallerCache = caller;
         }
         // Run constructor
         try {
--- a/src/java.base/share/classes/java/lang/reflect/Constructor.java	Tue May 12 07:55:15 2015 +0100
+++ b/src/java.base/share/classes/java/lang/reflect/Constructor.java	Tue May 12 11:40:05 2015 +0100
@@ -410,10 +410,8 @@
                IllegalArgumentException, InvocationTargetException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, null, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, null, modifiers);
         }
         if ((clazz.getModifiers() & Modifier.ENUM) != 0)
             throw new IllegalArgumentException("Cannot reflectively create enum objects");
--- a/src/java.base/share/classes/java/lang/reflect/Field.java	Tue May 12 07:55:15 2015 +0100
+++ b/src/java.base/share/classes/java/lang/reflect/Field.java	Tue May 12 11:40:05 2015 +0100
@@ -385,10 +385,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).get(obj);
     }
@@ -420,10 +418,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).getBoolean(obj);
     }
@@ -455,10 +451,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).getByte(obj);
     }
@@ -492,10 +486,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).getChar(obj);
     }
@@ -529,10 +521,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).getShort(obj);
     }
@@ -566,10 +556,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).getInt(obj);
     }
@@ -603,10 +591,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).getLong(obj);
     }
@@ -640,10 +626,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).getFloat(obj);
     }
@@ -677,10 +661,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         return getFieldAccessor(obj).getDouble(obj);
     }
@@ -756,10 +738,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).set(obj, value);
     }
@@ -793,10 +773,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).setBoolean(obj, z);
     }
@@ -830,10 +808,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).setByte(obj, b);
     }
@@ -867,10 +843,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).setChar(obj, c);
     }
@@ -904,10 +878,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).setShort(obj, s);
     }
@@ -941,10 +913,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).setInt(obj, i);
     }
@@ -978,10 +948,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).setLong(obj, l);
     }
@@ -1015,10 +983,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).setFloat(obj, f);
     }
@@ -1052,10 +1018,8 @@
         throws IllegalArgumentException, IllegalAccessException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         getFieldAccessor(obj).setDouble(obj, d);
     }
--- a/src/java.base/share/classes/java/lang/reflect/Method.java	Tue May 12 07:55:15 2015 +0100
+++ b/src/java.base/share/classes/java/lang/reflect/Method.java	Tue May 12 11:40:05 2015 +0100
@@ -490,10 +490,8 @@
            InvocationTargetException
     {
         if (!override) {
-            if (!Reflection.quickCheckMemberAccess(clazz, modifiers)) {
-                Class<?> caller = Reflection.getCallerClass();
-                checkAccess(caller, clazz, obj, modifiers);
-            }
+            Class<?> caller = Reflection.getCallerClass();
+            checkAccess(caller, clazz, obj, modifiers);
         }
         MethodAccessor ma = methodAccessor;             // read volatile
         if (ma == null) {
--- a/src/java.base/share/classes/java/lang/reflect/Module.java	Tue May 12 07:55:15 2015 +0100
+++ b/src/java.base/share/classes/java/lang/reflect/Module.java	Tue May 12 11:40:05 2015 +0100
@@ -55,12 +55,33 @@
 import sun.misc.Unsafe;
 
 /**
- * Represents a runtime module.
+ * Represents a run-time module, either named or {@link #isUnnamed() unnamed}.
  *
- * <p> {@code Module} does not define a public constructor. Instead {@code
- * Module} objects are constructed by the Java Virtual Machine when a
- * {@link java.lang.module.Configuration Configuration} is reified by means
- * of creating a {@link java.lang.module.Layer Layer}. </p>
+ * <p> Named modules have a {@link #getName() name} and are constructed by the
+ * Java Virtual Machine when a {@link java.lang.module.Configuration
+ * Configuration} is reified by creating a module {@link java.lang.module.Layer
+ * Layer}. </p>
+ *
+ * <p> An unnamed module does not have a name. There is an unnamed module
+ * per {@link ClassLoader ClassLoader} that is obtained by invoking the class
+ * loader's {@link ClassLoader#getUnnamedModule() getUnnamedModule} method. The
+ * {@link Class#getModule() getModule} method of all types defined by a class
+ * loader that are not in a named module return the class loader's unnamed
+ * module. An unnamed module reads all other run-time modules. </p>
+ *
+ * <p> A named module is either {@link #isStrict strict} or <em>loose</em>.
+ * A strict module does not read arbitrary unnamed modules and so can not
+ * access all types defined by arbitrary class loaders in its unnamed module
+ * (even if the type is public). On the other hand, a loose module reads all
+ * unnamed modules and thus code in a loose module may access all public types
+ * in any unnamed module (assuming the code in the loose modules can get a
+ * reference to the type).
+ * A named module is always created as a strict module but may be changed to a
+ * loose module by invoking its {@link #addReads(Module) addReads} method with
+ * a target module of {@code null}. Modules that are frameworks or libraries
+ * that instantiate or introspect arbitrary {@code Class} objects are examples
+ * of modules that may change themselves from strict to loose. An unnamed
+ * module is always a loose module. </p>
  *
  * @since 1.9
  * @see java.lang.Class#getModule
@@ -146,6 +167,22 @@
     }
 
     /**
+     * Returns {@code true} if this module is an unnamed module.
+     *
+     * @see ClassLoader#getUnnamedModule()
+     */
+    public boolean isUnnamed() {
+        return name == null;
+    }
+
+    /**
+     * Returns {@code true} if this module is a strict module.
+     */
+    public boolean isStrict() {
+        return !loose;
+    }
+
+    /**
      * Returns the {@code ClassLoader} for this module.
      *
      * <p> If there is a security manager then its {@code checkPermission}
@@ -166,24 +203,6 @@
     }
 
     /**
-     * Returns {@code true} if this module is an unnamed module.
-     *
-     * @see ClassLoader#getUnnamedModule()
-     */
-    public boolean isUnnamed() {
-        return name == null;
-    }
-
-    /**
-     * Returns {@code true} if this module is a strict module.
-     * A strict module does not read all unnamed modules.
-     */
-    public boolean isStrict() {
-        return !loose;
-    }
-
-
-    /**
      * Returns the module descriptor for this module.
      *
      * For now, this method returns {@code null} if this module is an
@@ -194,6 +213,13 @@
     }
 
     /**
+     * @apiNote Need to decide whether to add this method as a public method.
+     */
+    Layer getLayer() {
+        throw new RuntimeException();
+    }
+
+    /**
      * Returns an array of the package names of the packages in this module.
      *
      * <p> The package names are the fully-qualified names of the packages as
@@ -226,12 +252,13 @@
 
     /**
      * Makes the given {@code Module} readable to this module. This method
-     * is no-op if {@code target} is {@code this} module (all modules
-     * can read themselves).
+     * is a no-op if {@code target} is {@code this} module (all modules
+     * can read themselves) or this module is an {@link #isUnnamed() unnamed}
+     * module (an unnamed module reads all other modules).
      *
      * <p> If {@code target} is {@code null}, and this module is a {@link
-     * #isStrict strict} module, the method changes this module to be a
-     * <em>loose module</em>. A loose module all unnamed modules. </p>
+     * #isStrict strict} module, then this method changes this module to be a
+     * <em>loose module</em>. A loose module reads all unnamed modules. </p>
      *
      * <p> If there is a security manager then its {@code checkPermission}
      * method if first called with a {@code ReflectPermission("addReadsModule")}
@@ -243,7 +270,7 @@
      * @see #canRead
      */
     public void addReads(Module target) {
-        if (target != this) {
+        if (target != this && !this.isUnnamed()) {
             SecurityManager sm = System.getSecurityManager();
             if (sm != null) {
                 ReflectPermission perm = new ReflectPermission("addReadsModule");
@@ -695,18 +722,21 @@
      * Returns an input stream for reading a resource in this module. Returns
      * {@code null} if the resource is not in this module or access to the
      * resource is denied by the security manager.
-     *
      * The {@code name} is a {@code '/'}-separated path name that identifies
      * the resource.
      *
+     * <p> If this module is an unnamed module, and the {@code ClassLoader} for
+     * this module is not {@code null}, then this method is equivalent to
+     * invoking the {@link ClassLoader#getResourceAsStream(String)
+     * getResourceAsStream} method on the class loader for this module.
+     *
      * @throws IOException
      *         If an I/O error occurs
      */
     public InputStream getResourceAsStream(String name) throws IOException {
         Objects.requireNonNull(name);
 
-        // if called on an unnamed module then work the same way as
-        // ClassLoader::getResource.
+        // unnamed module
         if (isUnnamed()) {
             URL url;
             if (loader == null) {
@@ -721,6 +751,7 @@
             }
         }
 
+        // named module
         if (loader == null) {
             return BootLoader.getResourceAsStream(this.name, name);
         } else {
--- a/src/java.base/share/classes/sun/reflect/Reflection.java	Tue May 12 07:55:15 2015 +0100
+++ b/src/java.base/share/classes/sun/reflect/Reflection.java	Tue May 12 11:40:05 2015 +0100
@@ -81,13 +81,6 @@
         valid. */
     public static native int getClassAccessFlags(Class<?> c);
 
-    /** A quick "fast-path" check to try to avoid getCallerClass()
-        calls. No longer effective and so always returns false. */
-    public static boolean quickCheckMemberAccess(Class<?> memberClass,
-                                                 int modifiers)
-    {
-        return false;
-    }
 
     public static void ensureMemberAccess(Class<?> currentClass,
                                           Class<?> memberClass,
--- a/test/jdk/jigsaw/reflect/readable/src/m1/one/Main.java	Tue May 12 07:55:15 2015 +0100
+++ b/test/jdk/jigsaw/reflect/readable/src/m1/one/Main.java	Tue May 12 11:40:05 2015 +0100
@@ -44,9 +44,6 @@
         m1.addReads(m1); // no-op
         assertTrue(m1.canRead(m1));
 
-        // unnamed module is readable
-        //assertTrue(m1.canRead(null));
-
         // module m2 is not readable
         Class<?> c = Class.forName("jdk.two.C");
         Module m2 = c.getModule();