7103570: AtomicIntegerFieldUpdater does not work when SecurityManager is installed

Summary: Perform class.getField inside a doPrivileged block
Reviewed-by: chegar, psandoz

src/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java

@@ -35,7 +35,11 @@
 
 package java.util.concurrent.atomic;
 import sun.misc.Unsafe;
-import java.lang.reflect.*;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.security.AccessController;
+import java.security.PrivilegedExceptionAction;
+import java.security.PrivilegedActionException;
 
 /**
  * A reflection-based utility that enables atomic updates to
@@ -67,7 +71,9 @@ public abstract class  AtomicIntegerFieldUpdater<T> {
      * @throws IllegalArgumentException if the field is not a
      * volatile integer type
      * @throws RuntimeException with a nested reflection-based
-     * exception if the class does not hold field or is the wrong type
+     * exception if the class does not hold field or is the wrong type,
+     * or the field is inaccessible to the caller according to Java language
+     * access control
      */
     public static <U> AtomicIntegerFieldUpdater<U> newUpdater(Class<U> tclass, String fieldName) {
         return new AtomicIntegerFieldUpdaterImpl<U>(tclass, fieldName);
@@ -267,17 +273,29 @@ public abstract class  AtomicIntegerFieldUpdater<T> {
         private final Class<T> tclass;
         private final Class<?> cclass;
 
-        AtomicIntegerFieldUpdaterImpl(Class<T> tclass, String fieldName) {
+        AtomicIntegerFieldUpdaterImpl(final Class<T> tclass, final String fieldName) {
             Field field = null;
             Class<?> caller = null;
             int modifiers = 0;
             try {
-                field = tclass.getDeclaredField(fieldName);
+                field = AccessController.doPrivileged(
+                    new PrivilegedExceptionAction<Field>() {
+                        public Field run() throws NoSuchFieldException {
+                            return tclass.getDeclaredField(fieldName);
+                        }
+                    });
                 caller = sun.reflect.Reflection.getCallerClass(3);
                 modifiers = field.getModifiers();
                 sun.reflect.misc.ReflectUtil.ensureMemberAccess(
                     caller, tclass, null, modifiers);
-                sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                ClassLoader cl = tclass.getClassLoader();
+                ClassLoader ccl = caller.getClassLoader();
+                if ((ccl != null) && (ccl != cl) &&
+                    ((cl == null) || !isAncestor(cl, ccl))) {
+                  sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                }
+            } catch (PrivilegedActionException pae) {
+                throw new RuntimeException(pae.getException());
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             }
@@ -295,6 +313,22 @@ public abstract class  AtomicIntegerFieldUpdater<T> {
             offset = unsafe.objectFieldOffset(field);
         }
 
+        /**
+         * Returns true if the second classloader can be found in the first
+         * classloader's delegation chain.
+         * Equivalent to the inaccessible: first.isAncestor(second).
+         */
+        private static boolean isAncestor(ClassLoader first, ClassLoader second) {
+            ClassLoader acl = first;
+            do {
+                acl = acl.getParent();
+                if (second == acl) {
+                    return true;
+                }
+            } while (acl != null);
+            return false;
+        }
+
         private void fullCheck(T obj) {
             if (!tclass.isInstance(obj))
                 throw new ClassCastException();

src/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java

@@ -35,7 +35,11 @@
 
 package java.util.concurrent.atomic;
 import sun.misc.Unsafe;
-import java.lang.reflect.*;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.security.AccessController;
+import java.security.PrivilegedExceptionAction;
+import java.security.PrivilegedActionException;
 
 /**
  * A reflection-based utility that enables atomic updates to
@@ -67,7 +71,9 @@ public abstract class  AtomicLongFieldUpdater<T> {
      * @throws IllegalArgumentException if the field is not a
      * volatile long type.
      * @throws RuntimeException with a nested reflection-based
-     * exception if the class does not hold field or is the wrong type.
+     * exception if the class does not hold field or is the wrong type,
+     * or the field is inaccessible to the caller according to Java language
+     * access control
      */
     public static <U> AtomicLongFieldUpdater<U> newUpdater(Class<U> tclass, String fieldName) {
         if (AtomicLong.VM_SUPPORTS_LONG_CAS)
@@ -267,17 +273,29 @@ public abstract class  AtomicLongFieldUpdater<T> {
         private final Class<T> tclass;
         private final Class<?> cclass;
 
-        CASUpdater(Class<T> tclass, String fieldName) {
+        CASUpdater(final Class<T> tclass, final String fieldName) {
             Field field = null;
             Class<?> caller = null;
             int modifiers = 0;
             try {
-                field = tclass.getDeclaredField(fieldName);
+                field = AccessController.doPrivileged(
+                    new PrivilegedExceptionAction<Field>() {
+                        public Field run() throws NoSuchFieldException {
+                            return tclass.getDeclaredField(fieldName);
+                        }
+                    });
                 caller = sun.reflect.Reflection.getCallerClass(3);
                 modifiers = field.getModifiers();
                 sun.reflect.misc.ReflectUtil.ensureMemberAccess(
                     caller, tclass, null, modifiers);
-                sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                ClassLoader cl = tclass.getClassLoader();
+                ClassLoader ccl = caller.getClassLoader();
+                if ((ccl != null) && (ccl != cl) &&
+                    ((cl == null) || !isAncestor(cl, ccl))) {
+                  sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                }
+            } catch (PrivilegedActionException pae) {
+                throw new RuntimeException(pae.getException());
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             }
@@ -350,17 +368,29 @@ public abstract class  AtomicLongFieldUpdater<T> {
         private final Class<T> tclass;
         private final Class<?> cclass;
 
-        LockedUpdater(Class<T> tclass, String fieldName) {
+        LockedUpdater(final Class<T> tclass, final String fieldName) {
             Field field = null;
             Class<?> caller = null;
             int modifiers = 0;
             try {
-                field = tclass.getDeclaredField(fieldName);
+                field = AccessController.doPrivileged(
+                    new PrivilegedExceptionAction<Field>() {
+                        public Field run() throws NoSuchFieldException {
+                            return tclass.getDeclaredField(fieldName);
+                        }
+                    });
                 caller = sun.reflect.Reflection.getCallerClass(3);
                 modifiers = field.getModifiers();
                 sun.reflect.misc.ReflectUtil.ensureMemberAccess(
                     caller, tclass, null, modifiers);
-                sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                ClassLoader cl = tclass.getClassLoader();
+                ClassLoader ccl = caller.getClassLoader();
+                if ((ccl != null) && (ccl != cl) &&
+                    ((cl == null) || !isAncestor(cl, ccl))) {
+                  sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                }
+            } catch (PrivilegedActionException pae) {
+                throw new RuntimeException(pae.getException());
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             }
@@ -433,4 +463,20 @@ public abstract class  AtomicLongFieldUpdater<T> {
             );
         }
     }
+
+    /**
+     * Returns true if the second classloader can be found in the first
+     * classloader's delegation chain.
+     * Equivalent to the inaccessible: first.isAncestor(second).
+     */
+    private static boolean isAncestor(ClassLoader first, ClassLoader second) {
+        ClassLoader acl = first;
+        do {
+            acl = acl.getParent();
+            if (second == acl) {
+                return true;
+            }
+        } while (acl != null);
+        return false;
+    }
 }

src/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java

@@ -35,7 +35,11 @@
 
 package java.util.concurrent.atomic;
 import sun.misc.Unsafe;
-import java.lang.reflect.*;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.security.AccessController;
+import java.security.PrivilegedExceptionAction;
+import java.security.PrivilegedActionException;
 
 /**
  * A reflection-based utility that enables atomic updates to
@@ -86,7 +90,9 @@ public abstract class AtomicReferenceFieldUpdater<T, V> {
      * @return the updater
      * @throws IllegalArgumentException if the field is not a volatile reference type.
      * @throws RuntimeException with a nested reflection-based
-     * exception if the class does not hold field or is the wrong type.
+     * exception if the class does not hold field or is the wrong type,
+     * or the field is inaccessible to the caller according to Java language
+     * access control
      */
     public static <U, W> AtomicReferenceFieldUpdater<U,W> newUpdater(Class<U> tclass, Class<W> vclass, String fieldName) {
         return new AtomicReferenceFieldUpdaterImpl<U,W>(tclass,
@@ -197,21 +203,33 @@ public abstract class AtomicReferenceFieldUpdater<T, V> {
          * screenings fail.
          */
 
-        AtomicReferenceFieldUpdaterImpl(Class<T> tclass,
+        AtomicReferenceFieldUpdaterImpl(final Class<T> tclass,
                                         Class<V> vclass,
-                                        String fieldName) {
+                                        final String fieldName) {
             Field field = null;
             Class<?> fieldClass = null;
             Class<?> caller = null;
             int modifiers = 0;
             try {
-                field = tclass.getDeclaredField(fieldName);
+                field = AccessController.doPrivileged(
+                    new PrivilegedExceptionAction<Field>() {
+                        public Field run() throws NoSuchFieldException {
+                            return tclass.getDeclaredField(fieldName);
+                        }
+                    });
                 caller = sun.reflect.Reflection.getCallerClass(3);
                 modifiers = field.getModifiers();
                 sun.reflect.misc.ReflectUtil.ensureMemberAccess(
-                    caller, tclass, null, modifiers);
-                sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                                                                caller, tclass, null, modifiers);
+                ClassLoader cl = tclass.getClassLoader();
+                ClassLoader ccl = caller.getClassLoader();
+                if ((ccl != null) && (ccl != cl) &&
+                    ((cl == null) || !isAncestor(cl, ccl))) {
+                    sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                }
                 fieldClass = field.getType();
+            } catch (PrivilegedActionException pae) {
+                throw new RuntimeException(pae.getException());
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             }
@@ -232,6 +250,22 @@ public abstract class AtomicReferenceFieldUpdater<T, V> {
             offset = unsafe.objectFieldOffset(field);
         }
 
+        /**
+         * Returns true if the second classloader can be found in the first
+         * classloader's delegation chain.
+         * Equivalent to the inaccessible: first.isAncestor(second).
+         */
+        private static boolean isAncestor(ClassLoader first, ClassLoader second) {
+            ClassLoader acl = first;
+            do {
+                acl = acl.getParent();
+                if (second == acl) {
+                    return true;
+                }
+            } while (acl != null);
+            return false;
+        }
+
         void targetCheck(T obj) {
             if (!tclass.isInstance(obj))
                 throw new ClassCastException();
@@ -281,7 +315,7 @@ public abstract class AtomicReferenceFieldUpdater<T, V> {
         }
 
         @SuppressWarnings("unchecked")
-        public V get(T obj) {
+            public V get(T obj) {
             if (obj == null || obj.getClass() != tclass || cclass != null)
                 targetCheck(obj);
             return (V)unsafe.getObjectVolatile(obj, offset);
@@ -292,14 +326,14 @@ public abstract class AtomicReferenceFieldUpdater<T, V> {
                 return;
             }
             throw new RuntimeException(
-                new IllegalAccessException("Class " +
-                    cclass.getName() +
-                    " can not access a protected member of class " +
-                    tclass.getName() +
-                    " using an instance of " +
-                    obj.getClass().getName()
-                )
-            );
+                                       new IllegalAccessException("Class " +
+                                                                  cclass.getName() +
+                                                                  " can not access a protected member of class " +
+                                                                  tclass.getName() +
+                                                                  " using an instance of " +
+                                                                  obj.getClass().getName()
+                                                                  )
+                                       );
         }
     }
 }

test/java/util/concurrent/atomic/AtomicUpdaters.java

+/*
+ * Copyright (c) 2012, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 7103570
+ * @author David Holmes
+ * @run main/othervm AtomicUpdaters
+ * @run main/othervm AtomicUpdaters UseSM
+ * @summary Checks the (in)ability to create field updaters for differently
+ *          accessible fields in different locations with/without a security
+ *          manager
+ */
+import java.util.concurrent.atomic.*;
+import java.lang.reflect.*;
+import java.security.AccessControlException;
+
+public class  AtomicUpdaters {
+    enum TYPE { INT, LONG, REF }
+
+    static class Config {
+        final Class<?> clazz;
+        final String field;
+        final String access;
+        final boolean reflectOk;
+        final boolean updaterOk;
+        final String desc;
+        final TYPE type;
+
+        Config(Class<?> clazz, String field, String access,
+               boolean reflectOk, boolean updaterOk, String desc, TYPE type) {
+            this.clazz = clazz;
+            this.field = field;
+            this.access = access;
+            this.reflectOk = reflectOk;
+            this.updaterOk = updaterOk;
+            this.desc = desc;
+            this.type =type;
+        }
+
+        public String toString() {
+            return desc + ": " + access + " " + clazz.getName() + "." + field;
+        }
+    }
+
+    static Config[] tests;
+
+    static void initTests(boolean hasSM) {
+        tests = new Config[] {
+            new Config(AtomicUpdaters.class, "pub_int", "public", true, true, "public int field of current class", TYPE.INT),
+            new Config(AtomicUpdaters.class, "priv_int", "private", true, true, "private int field of current class", TYPE.INT),
+            new Config(AtomicUpdaters.class, "pub_long", "public", true, true, "public long field of current class", TYPE.LONG),
+            new Config(AtomicUpdaters.class, "priv_long", "private", true, true, "private long field of current class", TYPE.LONG),
+            new Config(AtomicUpdaters.class, "pub_ref", "public", true, true, "public ref field of current class", TYPE.REF),
+            new Config(AtomicUpdaters.class, "priv_ref", "private", true, true, "private ref field of current class", TYPE.REF),
+
+            // Would like to test a public volatile in a class in another
+            // package - but of course there aren't any
+            new Config(java.util.concurrent.atomic.AtomicInteger.class, "value", "private", hasSM ? false : true, false, "private int field of class in different package", TYPE.INT),
+            new Config(java.util.concurrent.atomic.AtomicLong.class, "value", "private", hasSM ? false : true, false, "private long field of class in different package", TYPE.LONG),
+            new Config(java.util.concurrent.atomic.AtomicReference.class, "value", "private", hasSM ? false : true, false, "private reference field of class in different package", TYPE.REF),
+        };
+    }
+
+    public volatile int pub_int;
+    private volatile int priv_int;
+    public volatile long pub_long;
+    private volatile long priv_long;
+    public volatile Object pub_ref;
+    private volatile Object priv_ref;
+
+
+    // This should be set dynamically at runtime using a System property, but
+    // ironically we get a SecurityException if we try to do that with a
+    // SecurityManager installed
+    static boolean verbose;
+
+    public static void main(String[] args) throws Throwable {
+        boolean hasSM = false;
+        for (String arg : args) {
+            if ("-v".equals(arg)) {
+                verbose = true;
+            }
+            else if ("UseSM".equals(arg)) {
+                SecurityManager m = System.getSecurityManager();
+                if (m != null)
+                    throw new RuntimeException("No security manager should initially be installed");
+                System.setSecurityManager(new java.lang.SecurityManager());
+                hasSM = true;
+            }
+            else {
+                throw new IllegalArgumentException("Unexpected option: " + arg);
+            }
+        }
+        initTests(hasSM);
+
+        int failures = 0;
+
+        System.out.printf("Testing with%s a SecurityManager present\n", hasSM ? "" : "out");
+        for (Config c : tests) {
+            System.out.println("Testing: " + c);
+            Error reflectionFailure = null;
+            Error updaterFailure = null;
+            Class<?> clazz = c.clazz;
+            // See if we can reflectively access the field
+            System.out.println(" - testing getDeclaredField");
+            try {
+                Field f = clazz.getDeclaredField(c.field);
+                if (!c.reflectOk)
+                    reflectionFailure = new Error("Unexpected reflective access: " + c);
+            }
+            catch (AccessControlException e) {
+                if (c.reflectOk)
+                    reflectionFailure = new Error("Unexpected reflective access failure: " + c, e);
+                else if (verbose) {
+                    System.out.println("Got expected reflection exception: " + e);
+                    e.printStackTrace(System.out);
+                }
+            }
+
+            if (reflectionFailure != null) {
+                reflectionFailure.printStackTrace(System.out);
+            }
+
+            // see if we can create an atomic updater for the field
+            Object u = null;
+            try {
+                switch (c.type) {
+                case INT:
+                    System.out.println(" - testing AtomicIntegerFieldUpdater");
+                    u = AtomicIntegerFieldUpdater.newUpdater(clazz, c.field);
+                    break;
+                case LONG:
+                    System.out.println(" - testing AtomicLongFieldUpdater");
+                    u = AtomicLongFieldUpdater.newUpdater(clazz, c.field);
+                    break;
+                case REF:
+                    System.out.println(" - testing AtomicReferenceFieldUpdater");
+                    u = AtomicReferenceFieldUpdater.newUpdater(clazz, Object.class, c.field);
+                    break;
+                }
+
+                if (!c.updaterOk)
+                    updaterFailure =  new Error("Unexpected updater access: " + c);
+            }
+            catch (Exception e) {
+                if (c.updaterOk)
+                    updaterFailure = new Error("Unexpected updater access failure: " + c, e);
+                else if (verbose) {
+                    System.out.println("Got expected updater exception: " + e);
+                    e.printStackTrace(System.out);
+                }
+            }
+
+            if (updaterFailure != null) {
+                updaterFailure.printStackTrace(System.out);
+            }
+
+            if (updaterFailure != null || reflectionFailure != null) {
+                failures++;
+
+            }
+        }
+
+        if (failures > 0) {
+            throw new Error("Some tests failed - see previous stacktraces");
+        }
+    }
+}