OpenJDK / jdk / jdk12

changeset 34339:61d78c23fcdc

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8140587: Atomic*FieldUpdaters should use Class.isInstance instead of direct class check Reviewed-by: martin, psandoz, chegar, shade, plevart
author dl
date Wed, 25 Nov 2015 18:12:38 -0800
parents 67d0e5867568
children 3b7a5a01c627
files jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java
diffstat 3 files changed, 215 insertions(+), 194 deletions(-) [+]
line wrap: on
line diff
--- a/jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java	Wed Nov 25 15:36:03 2015 -0500
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java	Wed Nov 25 18:12:38 2015 -0800
@@ -365,12 +365,17 @@
     /**
      * Standard hotspot implementation using intrinsics.
      */
-    private static class AtomicIntegerFieldUpdaterImpl<T>
-            extends AtomicIntegerFieldUpdater<T> {
+    private static final class AtomicIntegerFieldUpdaterImpl<T>
+        extends AtomicIntegerFieldUpdater<T> {
         private static final jdk.internal.misc.Unsafe U = jdk.internal.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,
@@ -399,17 +404,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 = U.objectFieldOffset(field);
+            this.offset = U.objectFieldOffset(field);
         }
 
         /**
@@ -428,81 +431,87 @@
             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);
+        /**
+         * 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 final boolean compareAndSet(T obj, int expect, int update) {
+            accessCheck(obj);
             return U.compareAndSwapInt(obj, offset, expect, update);
         }
 
-        public boolean weakCompareAndSet(T obj, int expect, int update) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        public final boolean weakCompareAndSet(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);
+        public final void set(T obj, int newValue) {
+            accessCheck(obj);
             U.putIntVolatile(obj, offset, newValue);
         }
 
-        public void lazySet(T obj, int newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        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);
+            accessCheck(obj);
             return U.getIntVolatile(obj, offset);
         }
 
-        public int getAndSet(T obj, int newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        public final int getAndSet(T obj, int newValue) {
+            accessCheck(obj);
             return U.getAndSetInt(obj, offset, newValue);
         }
 
-        public int getAndIncrement(T obj) {
+        public final int getAndAdd(T obj, int delta) {
+            accessCheck(obj);
+            return U.getAndAddInt(obj, offset, delta);
+        }
+
+        public final int getAndIncrement(T obj) {
             return getAndAdd(obj, 1);
         }
 
-        public int getAndDecrement(T obj) {
+        public final int getAndDecrement(T obj) {
             return getAndAdd(obj, -1);
         }
 
-        public int getAndAdd(T obj, int delta) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            return U.getAndAddInt(obj, offset, delta);
-        }
-
-        public int incrementAndGet(T obj) {
+        public final int incrementAndGet(T obj) {
             return getAndAdd(obj, 1) + 1;
         }
 
-        public int decrementAndGet(T obj) {
+        public final int decrementAndGet(T obj) {
             return getAndAdd(obj, -1) - 1;
         }
 
-        public int addAndGet(T obj, int delta) {
+        public final int addAndGet(T obj, int delta) {
             return getAndAdd(obj, delta) + delta;
         }
 
-        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/jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java	Wed Nov 25 15:36:03 2015 -0500
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java	Wed Nov 25 18:12:38 2015 -0800
@@ -365,11 +365,16 @@
         return next;
     }
 
-    private static class CASUpdater<T> extends AtomicLongFieldUpdater<T> {
+    private static final class CASUpdater<T> extends AtomicLongFieldUpdater<T> {
         private static final jdk.internal.misc.Unsafe U = jdk.internal.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,
                    final Class<?> caller) {
@@ -397,103 +402,110 @@
                 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 = U.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);
+        /**
+         * 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 final boolean compareAndSet(T obj, long expect, long update) {
+            accessCheck(obj);
             return U.compareAndSwapLong(obj, offset, expect, update);
         }
 
-        public boolean weakCompareAndSet(T obj, long expect, long update) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        public final boolean weakCompareAndSet(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);
+        public final void set(T obj, long newValue) {
+            accessCheck(obj);
             U.putLongVolatile(obj, offset, newValue);
         }
 
-        public void lazySet(T obj, long newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        public final void lazySet(T obj, long newValue) {
+            accessCheck(obj);
             U.putOrderedLong(obj, offset, newValue);
         }
 
-        public long get(T obj) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        public final long get(T obj) {
+            accessCheck(obj);
             return U.getLongVolatile(obj, offset);
         }
 
-        public long getAndSet(T obj, long newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+        public final long getAndSet(T obj, long newValue) {
+            accessCheck(obj);
             return U.getAndSetLong(obj, offset, newValue);
         }
 
-        public long getAndIncrement(T obj) {
+        public final long getAndAdd(T obj, long delta) {
+            accessCheck(obj);
+            return U.getAndAddLong(obj, offset, delta);
+        }
+
+        public final long getAndIncrement(T obj) {
             return getAndAdd(obj, 1);
         }
 
-        public long getAndDecrement(T obj) {
+        public final long getAndDecrement(T obj) {
             return getAndAdd(obj, -1);
         }
 
-        public long getAndAdd(T obj, long delta) {
-            if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
-            return U.getAndAddLong(obj, offset, delta);
-        }
-
-        public long incrementAndGet(T obj) {
+        public final long incrementAndGet(T obj) {
             return getAndAdd(obj, 1) + 1;
         }
 
-        public long decrementAndGet(T obj) {
+        public final long decrementAndGet(T obj) {
             return getAndAdd(obj, -1) - 1;
         }
 
-        public long addAndGet(T obj, long delta) {
+        public final long addAndGet(T obj, long delta) {
             return getAndAdd(obj, delta) + delta;
         }
-
-        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()
-                )
-            );
-        }
     }
 
-
-    private static class LockedUpdater<T> extends AtomicLongFieldUpdater<T> {
+    private static final class LockedUpdater<T> extends AtomicLongFieldUpdater<T> {
         private static final jdk.internal.misc.Unsafe U = jdk.internal.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,
                       final Class<?> caller) {
@@ -521,28 +533,46 @@
                 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 = U.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 = U.getLong(obj, offset);
                 if (v != expect)
@@ -552,42 +582,27 @@
             }
         }
 
-        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) {
                 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 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()
-                )
-            );
-        }
     }
 
     /**
--- a/jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java	Wed Nov 25 15:36:03 2015 -0500
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java	Wed Nov 25 18:12:38 2015 -0800
@@ -286,9 +286,15 @@
         extends AtomicReferenceFieldUpdater<T,V> {
         private static final jdk.internal.misc.Unsafe U = jdk.internal.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
@@ -340,14 +346,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 = U.objectFieldOffset(field);
+            this.vclass = vclass;
+            this.offset = U.objectFieldOffset(field);
         }
 
         /**
@@ -366,83 +368,78 @@
             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);
+        private final void valueCheck(V v) {
+            if (v != null && !(vclass.isInstance(v)))
+                throwCCE();
+        }
+
+        static void throwCCE() {
+            throw new ClassCastException();
+        }
+
+        public final boolean compareAndSet(T obj, V expect, V update) {
+            accessCheck(obj);
+            valueCheck(update);
             return U.compareAndSwapObject(obj, offset, expect, update);
         }
 
-        public boolean weakCompareAndSet(T obj, V expect, V update) {
+        public final 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);
+            accessCheck(obj);
+            valueCheck(update);
             return U.compareAndSwapObject(obj, offset, expect, update);
         }
 
-        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);
+        public final void set(T obj, V newValue) {
+            accessCheck(obj);
+            valueCheck(newValue);
             U.putObjectVolatile(obj, offset, newValue);
         }
 
-        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);
+        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);
+        public final V get(T obj) {
+            accessCheck(obj);
             return (V)U.getObjectVolatile(obj, offset);
         }
 
         @SuppressWarnings("unchecked")
-        public V getAndSet(T obj, V newValue) {
-            if (obj == null || obj.getClass() != tclass || cclass != null ||
-                (newValue != null && vclass != null &&
-                 vclass != newValue.getClass()))
-                updateCheck(obj, newValue);
+        public final V getAndSet(T obj, V newValue) {
+            accessCheck(obj);
+            valueCheck(newValue);
             return (V)U.getAndSetObject(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()
-                )
-            );
-        }
     }
 }
© 2007, 2022 Oracle and/or its affiliates
Terms of Use · Privacy · Trademarks