8140587: Atomic*FieldUpdaters should use Class.isInstance instead of direct class check
Reviewed-by: martin, psandoz, chegar, shade, plevart
--- 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) {
- return getAndAdd(obj, 1);
- }
-
- public 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);
+ public final int getAndAdd(T obj, int delta) {
+ accessCheck(obj);
return U.getAndAddInt(obj, offset, delta);
}
- public int incrementAndGet(T obj) {
+ public final int getAndIncrement(T obj) {
+ return getAndAdd(obj, 1);
+ }
+
+ public final int getAndDecrement(T obj) {
+ return getAndAdd(obj, -1);
+ }
+
+ 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);
+ }
+
+ /**
+ * Checks that target argument is instance of cclass. On
+ * failure, throws cause.
+ */
+ private final void accessCheck(T obj) {
+ if (!cclass.isInstance(obj))
+ throwAccessCheckException(obj);
}
- private void fullCheck(T obj) {
- if (!tclass.isInstance(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();
- 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, long expect, long update) {
- if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+ 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);
+ }
+
+ /**
+ * 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);
}
- private void fullCheck(T obj) {
- if (!tclass.isInstance(obj))
- throw new ClassCastException();
- if (cclass != null)
- ensureProtectedAccess(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 boolean compareAndSet(T obj, long expect, long update) {
- if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj);
+ 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()
- )
- );
- }
}
}