changeset 8607:738d6932b933

8140587: Atomic*FieldUpdaters should use Class.isInstance instead of direct class check Reviewed-by: martin
author robm
date Tue, 31 Jan 2017 23:36:58 +0000
parents 49b322942fbb
children 64257fdef625
files src/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java src/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java src/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java
diffstat 3 files changed, 223 insertions(+), 195 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java	Tue Jan 31 22:05:23 2017 +0000
+++ b/src/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java	Tue Jan 31 23:36:58 2017 +0000
@@ -34,11 +34,12 @@
  */
 
 package java.util.concurrent.atomic;
+
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.security.AccessController;
+import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
-import java.security.PrivilegedActionException;
 import sun.misc.Unsafe;
 import sun.reflect.CallerSensitive;
 import sun.reflect.Reflection;
@@ -268,13 +269,19 @@
     }
 
     /**
-     * Standard hotspot implementation using intrinsics
+     * Standard hotspot implementation using intrinsics.
      */
-    private static class AtomicIntegerFieldUpdaterImpl<T> extends AtomicIntegerFieldUpdater<T> {
-        private static final Unsafe unsafe = Unsafe.getUnsafe();
+    private static final class AtomicIntegerFieldUpdaterImpl<T>
+        extends AtomicIntegerFieldUpdater<T> {
+        private static final sun.misc.Unsafe U = sun.misc.Unsafe.getUnsafe();
         private final long offset;
+        /**
+         * if field is protected, the subclass constructing updater, else
+         * the same as tclass
+         */
+        private final Class<?> cclass;
+        /** class holding the field */
         private final Class<T> tclass;
-        private final Class<?> cclass;
 
         AtomicIntegerFieldUpdaterImpl(final Class<T> tclass, final String fieldName, Class<?> caller) {
             Field field = null;
@@ -293,7 +300,7 @@
                 ClassLoader ccl = caller.getClassLoader();
                 if ((ccl != null) && (ccl != cl) &&
                     ((cl == null) || !isAncestor(cl, ccl))) {
-                  sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                    sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
                 }
             } catch (PrivilegedActionException pae) {
                 throw new RuntimeException(pae.getException());
@@ -301,17 +308,15 @@
                 throw new RuntimeException(ex);
             }
 
-            Class<?> fieldt = field.getType();
-            if (fieldt != int.class)
+            if (field.getType() != int.class)
                 throw new IllegalArgumentException("Must be integer type");
 
             if (!Modifier.isVolatile(modifiers))
                 throw new IllegalArgumentException("Must be volatile type");
 
-            this.cclass = (Modifier.isProtected(modifiers) &&
-                           caller != tclass) ? caller : null;
+            this.cclass = (Modifier.isProtected(modifiers)) ? caller : tclass;
             this.tclass = tclass;
-            offset = unsafe.objectFieldOffset(field);
+            this.offset = U.objectFieldOffset(field);
         }
 
         /**
@@ -330,51 +335,57 @@
             return false;
         }
 
-        private void fullCheck(T obj) {
-            if (!tclass.isInstance(obj))
-                throw new ClassCastException();
-            if (cclass != null)
-                ensureProtectedAccess(obj);
+        /**
+         * Checks that target argument is instance of cclass.  On
+         * failure, throws cause.
+         */
+        private final void accessCheck(T obj) {
+            if (!cclass.isInstance(obj))
+                throwAccessCheckException(obj);
         }
 
-        public boolean compareAndSet(T obj, int expect, int update) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            return unsafe.compareAndSwapInt(obj, offset, expect, update);
+        /**
+         * Throws access exception if accessCheck failed due to
+         * protected access, else ClassCastException.
+         */
+        private final void throwAccessCheckException(T obj) {
+            if (cclass == tclass)
+                throw new ClassCastException();
+            else
+                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()));
         }
 
-        public boolean weakCompareAndSet(T obj, int expect, int update) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            return unsafe.compareAndSwapInt(obj, offset, expect, update);
+        public final boolean compareAndSet(T obj, int expect, int update) {
+            accessCheck(obj);
+            return U.compareAndSwapInt(obj, offset, expect, update);
         }
 
-        public void set(T obj, int newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            unsafe.putIntVolatile(obj, offset, newValue);
+        public final boolean weakCompareAndSet(T obj, int expect, int update) {
+            accessCheck(obj);
+            return U.compareAndSwapInt(obj, offset, expect, update);
         }
 
-        public void lazySet(T obj, int newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            unsafe.putOrderedInt(obj, offset, newValue);
+        public final void set(T obj, int newValue) {
+            accessCheck(obj);
+            U.putIntVolatile(obj, offset, newValue);
+        }
+
+        public final void lazySet(T obj, int newValue) {
+            accessCheck(obj);
+            U.putOrderedInt(obj, offset, newValue);
         }
 
         public final int get(T obj) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            return unsafe.getIntVolatile(obj, offset);
+            accessCheck(obj);
+            return U.getIntVolatile(obj, offset);
         }
 
-        private void ensureProtectedAccess(T obj) {
-            if (cclass.isInstance(obj)) {
-                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()
-                )
-            );
-        }
     }
 }
--- a/src/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java	Tue Jan 31 22:05:23 2017 +0000
+++ b/src/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java	Tue Jan 31 23:36:58 2017 +0000
@@ -34,11 +34,12 @@
  */
 
 package java.util.concurrent.atomic;
+
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.security.AccessController;
+import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
-import java.security.PrivilegedActionException;
 import sun.misc.Unsafe;
 import sun.reflect.CallerSensitive;
 import sun.reflect.Reflection;
@@ -271,11 +272,16 @@
         }
     }
 
-    private static class CASUpdater<T> extends AtomicLongFieldUpdater<T> {
-        private static final Unsafe unsafe = Unsafe.getUnsafe();
+    private static final class CASUpdater<T> extends AtomicLongFieldUpdater<T> {
+        private static final sun.misc.Unsafe U = sun.misc.Unsafe.getUnsafe();
         private final long offset;
+        /**
+         * if field is protected, the subclass constructing updater, else
+         * the same as tclass
+         */
+        private final Class<?> cclass;
+        /** class holding the field */
         private final Class<T> tclass;
-        private final Class<?> cclass;
 
         CASUpdater(final Class<T> tclass, final String fieldName, Class<?> caller) {
             Field field = null;
@@ -294,7 +300,7 @@
                 ClassLoader ccl = caller.getClassLoader();
                 if ((ccl != null) && (ccl != cl) &&
                     ((cl == null) || !isAncestor(cl, ccl))) {
-                  sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                    sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
                 }
             } catch (PrivilegedActionException pae) {
                 throw new RuntimeException(pae.getException());
@@ -302,73 +308,80 @@
                 throw new RuntimeException(ex);
             }
 
-            Class<?> fieldt = field.getType();
-            if (fieldt != long.class)
+            if (field.getType() != long.class)
                 throw new IllegalArgumentException("Must be long type");
 
             if (!Modifier.isVolatile(modifiers))
                 throw new IllegalArgumentException("Must be volatile type");
 
-            this.cclass = (Modifier.isProtected(modifiers) &&
-                           caller != tclass) ? caller : null;
+            this.cclass = (Modifier.isProtected(modifiers)) ? caller : tclass;
             this.tclass = tclass;
-            offset = unsafe.objectFieldOffset(field);
+            this.offset = U.objectFieldOffset(field);
         }
 
-        private void fullCheck(T obj) {
-            if (!tclass.isInstance(obj))
-                throw new ClassCastException();
-            if (cclass != null)
-                ensureProtectedAccess(obj);
+        /**
+         * Checks that target argument is instance of cclass.  On
+         * failure, throws cause.
+         */
+        private final void accessCheck(T obj) {
+            if (!cclass.isInstance(obj))
+                throwAccessCheckException(obj);
         }
 
-        public boolean compareAndSet(T obj, long expect, long update) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            return unsafe.compareAndSwapLong(obj, offset, expect, update);
+        /**
+         * Throws access exception if accessCheck failed due to
+         * protected access, else ClassCastException.
+         */
+        private final void throwAccessCheckException(T obj) {
+            if (cclass == tclass)
+                throw new ClassCastException();
+            else
+                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()));
         }
 
-        public boolean weakCompareAndSet(T obj, long expect, long update) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            return unsafe.compareAndSwapLong(obj, offset, expect, update);
+        public final boolean compareAndSet(T obj, long expect, long update) {
+            accessCheck(obj);
+            return U.compareAndSwapLong(obj, offset, expect, update);
         }
 
-        public void set(T obj, long newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            unsafe.putLongVolatile(obj, offset, newValue);
+        public final boolean weakCompareAndSet(T obj, long expect, long update) {
+            accessCheck(obj);
+            return U.compareAndSwapLong(obj, offset, expect, update);
         }
 
-        public void lazySet(T obj, long newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            unsafe.putOrderedLong(obj, offset, newValue);
+        public final void set(T obj, long newValue) {
+            accessCheck(obj);
+            U.putLongVolatile(obj, offset, newValue);
         }
 
-        public long get(T obj) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            return unsafe.getLongVolatile(obj, offset);
+        public final void lazySet(T obj, long newValue) {
+            accessCheck(obj);
+            U.putOrderedLong(obj, offset, newValue);
         }
 
-        private void ensureProtectedAccess(T obj) {
-            if (cclass.isInstance(obj)) {
-                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()
-                )
-            );
+        public final long get(T obj) {
+            accessCheck(obj);
+            return U.getLongVolatile(obj, offset);
         }
     }
 
-
-    private static class LockedUpdater<T> extends AtomicLongFieldUpdater<T> {
-        private static final Unsafe unsafe = Unsafe.getUnsafe();
+    private static final class LockedUpdater<T> extends AtomicLongFieldUpdater<T> {
+        private static final sun.misc.Unsafe U = sun.misc.Unsafe.getUnsafe();
         private final long offset;
+        /**
+         * if field is protected, the subclass constructing updater, else
+         * the same as tclass
+         */
+        private final Class<?> cclass;
+        /** class holding the field */
         private final Class<T> tclass;
-        private final Class<?> cclass;
 
         LockedUpdater(final Class<T> tclass, final String fieldName, Class<?> caller) {
             Field field = null;
@@ -387,7 +400,7 @@
                 ClassLoader ccl = caller.getClassLoader();
                 if ((ccl != null) && (ccl != cl) &&
                     ((cl == null) || !isAncestor(cl, ccl))) {
-                  sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+                    sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
                 }
             } catch (PrivilegedActionException pae) {
                 throw new RuntimeException(pae.getException());
@@ -395,73 +408,76 @@
                 throw new RuntimeException(ex);
             }
 
-            Class<?> fieldt = field.getType();
-            if (fieldt != long.class)
+            if (field.getType() != long.class)
                 throw new IllegalArgumentException("Must be long type");
 
             if (!Modifier.isVolatile(modifiers))
                 throw new IllegalArgumentException("Must be volatile type");
 
-            this.cclass = (Modifier.isProtected(modifiers) &&
-                           caller != tclass) ? caller : null;
+            this.cclass = (Modifier.isProtected(modifiers)) ? caller : tclass;
             this.tclass = tclass;
-            offset = unsafe.objectFieldOffset(field);
+            this.offset = U.objectFieldOffset(field);
         }
 
-        private void fullCheck(T obj) {
-            if (!tclass.isInstance(obj))
-                throw new ClassCastException();
-            if (cclass != null)
-                ensureProtectedAccess(obj);
+        /**
+         * Checks that target argument is instance of cclass.  On
+         * failure, throws cause.
+         */
+        private final void accessCheck(T obj) {
+            if (!cclass.isInstance(obj))
+                throw accessCheckException(obj);
         }
 
-        public boolean compareAndSet(T obj, long expect, long update) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        /**
+         * Returns access exception if accessCheck failed due to
+         * protected access, else ClassCastException.
+         */
+        private final RuntimeException accessCheckException(T obj) {
+            if (cclass == tclass)
+                return new ClassCastException();
+            else
+                return new RuntimeException(
+                    new IllegalAccessException(
+                        "Class " +
+                        cclass.getName() +
+                        " can not access a protected member of class " +
+                        tclass.getName() +
+                        " using an instance of " +
+                        obj.getClass().getName()));
+        }
+
+        public final boolean compareAndSet(T obj, long expect, long update) {
+            accessCheck(obj);
             synchronized (this) {
-                long v = unsafe.getLong(obj, offset);
+                long v = U.getLong(obj, offset);
                 if (v != expect)
                     return false;
-                unsafe.putLong(obj, offset, update);
+                U.putLong(obj, offset, update);
                 return true;
             }
         }
 
-        public boolean weakCompareAndSet(T obj, long expect, long update) {
+        public final boolean weakCompareAndSet(T obj, long expect, long update) {
             return compareAndSet(obj, expect, update);
         }
 
-        public void set(T obj, long newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        public final void set(T obj, long newValue) {
+            accessCheck(obj);
             synchronized (this) {
-                unsafe.putLong(obj, offset, newValue);
+                U.putLong(obj, offset, newValue);
             }
         }
 
-        public void lazySet(T obj, long newValue) {
+        public final void lazySet(T obj, long newValue) {
             set(obj, newValue);
         }
 
-        public long get(T obj) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        public final long get(T obj) {
+            accessCheck(obj);
             synchronized (this) {
-                return unsafe.getLong(obj, offset);
+                return U.getLong(obj, offset);
             }
         }
-
-        private void ensureProtectedAccess(T obj) {
-            if (cclass.isInstance(obj)) {
-                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()
-                )
-            );
-        }
     }
 
     /**
@@ -469,7 +485,7 @@
      * classloader's delegation chain.
      * Equivalent to the inaccessible: first.isAncestor(second).
      */
-    private static boolean isAncestor(ClassLoader first, ClassLoader second) {
+    static boolean isAncestor(ClassLoader first, ClassLoader second) {
         ClassLoader acl = first;
         do {
             acl = acl.getParent();
--- a/src/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java	Tue Jan 31 22:05:23 2017 +0000
+++ b/src/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java	Tue Jan 31 23:36:58 2017 +0000
@@ -34,11 +34,12 @@
  */
 
 package java.util.concurrent.atomic;
+
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.security.AccessController;
+import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
-import java.security.PrivilegedActionException;
 import sun.misc.Unsafe;
 import sun.reflect.CallerSensitive;
 import sun.reflect.Reflection;
@@ -51,7 +52,7 @@
  * independently subject to atomic updates. For example, a tree node
  * might be declared as
  *
- *  <pre> {@code
+ * <pre> {@code
  * class Node {
  *   private volatile Node left, right;
  *
@@ -60,7 +61,7 @@
  *   private static AtomicReferenceFieldUpdater<Node, Node> rightUpdater =
  *     AtomicReferenceFieldUpdater.newUpdater(Node.class, Node.class, "right");
  *
- *   Node getLeft() { return left;  }
+ *   Node getLeft() { return left; }
  *   boolean compareAndSetLeft(Node expect, Node update) {
  *     return leftUpdater.compareAndSet(this, expect, update);
  *   }
@@ -189,11 +190,17 @@
 
     private static final class AtomicReferenceFieldUpdaterImpl<T,V>
         extends AtomicReferenceFieldUpdater<T,V> {
-        private static final Unsafe unsafe = Unsafe.getUnsafe();
+        private static final sun.misc.Unsafe U = sun.misc.Unsafe.getUnsafe();
         private final long offset;
+        /**
+         * if field is protected, the subclass constructing updater, else
+         * the same as tclass
+         */
+        private final Class<?> cclass;
+        /** class holding the field */
         private final Class<T> tclass;
+        /** field value type */
         private final Class<V> vclass;
-        private final Class<?> cclass;
 
         /*
          * Internal type checks within all update methods contain
@@ -245,14 +252,10 @@
             if (!Modifier.isVolatile(modifiers))
                 throw new IllegalArgumentException("Must be volatile type");
 
-            this.cclass = (Modifier.isProtected(modifiers) &&
-                           caller != tclass) ? caller : null;
+            this.cclass = (Modifier.isProtected(modifiers)) ? caller : tclass;
             this.tclass = tclass;
-            if (vclass == Object.class)
-                this.vclass = null;
-            else
-                this.vclass = vclass;
-            offset = unsafe.objectFieldOffset(field);
+            this.vclass = vclass;
+            this.offset = U.objectFieldOffset(field);
         }
 
         /**
@@ -271,74 +274,72 @@
             return false;
         }
 
-        void targetCheck(T obj) {
-            if (!tclass.isInstance(obj))
-                throw new ClassCastException();
-            if (cclass != null)
-                ensureProtectedAccess(obj);
+        /**
+         * Checks that target argument is instance of cclass.  On
+         * failure, throws cause.
+         */
+        private final void accessCheck(T obj) {
+            if (!cclass.isInstance(obj))
+                throwAccessCheckException(obj);
         }
 
-        void updateCheck(T obj, V update) {
-            if (!tclass.isInstance(obj) ||
-                (update != null && vclass != null && !vclass.isInstance(update)))
+        /**
+         * Throws access exception if accessCheck failed due to
+         * protected access, else ClassCastException.
+         */
+        private final void throwAccessCheckException(T obj) {
+            if (cclass == tclass)
                 throw new ClassCastException();
-            if (cclass != null)
-                ensureProtectedAccess(obj);
+            else
+                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()));
         }
 
-        public boolean compareAndSet(T obj, V expect, V update) {
-            if (obj == null || obj.getClass() != tclass || cclass != null ||
-                (update != null && vclass != null &&
-                 vclass != update.getClass()))
-                updateCheck(obj, update);
-            return unsafe.compareAndSwapObject(obj, offset, expect, update);
+        private final void valueCheck(V v) {
+            if (v != null && !(vclass.isInstance(v)))
+                throwCCE();
         }
 
-        public boolean weakCompareAndSet(T obj, V expect, V update) {
-            // same implementation as strong form for now
-            if (obj == null || obj.getClass() != tclass || cclass != null ||
-                (update != null && vclass != null &&
-                 vclass != update.getClass()))
-                updateCheck(obj, update);
-            return unsafe.compareAndSwapObject(obj, offset, expect, update);
+        static void throwCCE() {
+            throw new ClassCastException();
         }
 
-        public void set(T obj, V newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null ||
-                (newValue != null && vclass != null &&
-                 vclass != newValue.getClass()))
-                updateCheck(obj, newValue);
-            unsafe.putObjectVolatile(obj, offset, newValue);
+        public final boolean compareAndSet(T obj, V expect, V update) {
+            accessCheck(obj);
+            valueCheck(update);
+            return U.compareAndSwapObject(obj, offset, expect, update);
         }
 
-        public void lazySet(T obj, V newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null ||
-                (newValue != null && vclass != null &&
-                 vclass != newValue.getClass()))
-                updateCheck(obj, newValue);
-            unsafe.putOrderedObject(obj, offset, newValue);
+        public final boolean weakCompareAndSet(T obj, V expect, V update) {
+            // same implementation as strong form for now
+            accessCheck(obj);
+            valueCheck(update);
+            return U.compareAndSwapObject(obj, offset, expect, update);
+        }
+
+        public final void set(T obj, V newValue) {
+            accessCheck(obj);
+            valueCheck(newValue);
+            U.putObjectVolatile(obj, offset, newValue);
+        }
+
+        public final void lazySet(T obj, V newValue) {
+            accessCheck(obj);
+            valueCheck(newValue);
+            U.putOrderedObject(obj, offset, newValue);
         }
 
         @SuppressWarnings("unchecked")
-            public V get(T obj) {
-            if (obj == null || obj.getClass() != tclass || cclass != null)
-                targetCheck(obj);
-            return (V)unsafe.getObjectVolatile(obj, offset);
+        public final V get(T obj) {
+            accessCheck(obj);
+            return (V)U.getObjectVolatile(obj, offset);
         }
 
-        private void ensureProtectedAccess(T obj) {
-            if (cclass.isInstance(obj)) {
-                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()
-                                                                  )
-                                       );
-        }
     }
 }