On different platforms AbstractFuture uses different strategies for its core synchronization * primitives. The strategies are all implemented as subtypes of AtomicHelper and the strategy is * selected in the static initializer of AbstractFuture. This is convenient and performant but - * introduces some testing difficulties. This test exercises the two fallback strategies in abstract - * future. - * - *
To force selection of our fallback strategies, we load {@link AbstractFuture} (and all of + * {@code com.google.common.util.concurrent}) in degenerate class loaders which make certain + * platform classes unavailable. Then we construct a test suite so we can run the normal + * AbstractFutureTest test methods in these degenerate classloaders. */ public class AbstractFutureFallbackAtomicHelperTest extends TestCase { @@ -60,7 +56,8 @@ public class AbstractFutureFallbackAtomicHelperTest extends TestCase { /** * This classloader disallows {@link sun.misc.Unsafe} and {@link AtomicReferenceFieldUpdater}, - * which will prevent us from selecting our {@code SafeAtomicHelper} strategy. + * which will prevent us from selecting our {@code AtomicReferenceFieldUpdaterAtomicHelper} + * strategy. */ @SuppressWarnings({"SunApi", "removal"}) // b/345822163 private static final ClassLoader NO_ATOMIC_REFERENCE_FIELD_UPDATER = @@ -142,4 +139,8 @@ public Class loadClass(String name) throws ClassNotFoundException { } }; } + + private static boolean isJava8() { + return JAVA_SPECIFICATION_VERSION.value().equals("1.8"); + } } diff --git a/guava-tests/test/com/google/common/util/concurrent/AbstractFutureFallbackAtomicHelperTest.java b/guava-tests/test/com/google/common/util/concurrent/AbstractFutureFallbackAtomicHelperTest.java index f106dee2e074..ed614a347c29 100644 --- a/guava-tests/test/com/google/common/util/concurrent/AbstractFutureFallbackAtomicHelperTest.java +++ b/guava-tests/test/com/google/common/util/concurrent/AbstractFutureFallbackAtomicHelperTest.java @@ -14,6 +14,8 @@ package com.google.common.util.concurrent; +import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION; + import com.google.common.collect.ImmutableSet; import java.lang.reflect.Field; import java.lang.reflect.Method; @@ -31,18 +33,12 @@ *
On different platforms AbstractFuture uses different strategies for its core synchronization * primitives. The strategies are all implemented as subtypes of AtomicHelper and the strategy is * selected in the static initializer of AbstractFuture. This is convenient and performant but - * introduces some testing difficulties. This test exercises the two fallback strategies in abstract - * future. - * - *
To force selection of our fallback strategies, we load {@link AbstractFuture} (and all of + * {@code com.google.common.util.concurrent}) in degenerate class loaders which make certain + * platform classes unavailable. Then we construct a test suite so we can run the normal + * AbstractFutureTest test methods in these degenerate classloaders. */ public class AbstractFutureFallbackAtomicHelperTest extends TestCase { @@ -51,21 +47,33 @@ public class AbstractFutureFallbackAtomicHelperTest extends TestCase { // execution significantly) /** - * This classloader disallows {@link sun.misc.Unsafe}, which will prevent us from selecting our - * preferred strategy {@code UnsafeAtomicHelper}. + * This classloader disallows {@link java.lang.invoke.VarHandle}, which will prevent us from + * selecting our preferred strategy {@code VarHandleAtomicHelper}. + */ + @SuppressWarnings({"SunApi", "removal"}) // b/345822163 + private static final ClassLoader NO_VAR_HANDLE = + getClassLoader(ImmutableSet.of("java.lang.invoke.VarHandle")); + + /** + * This classloader disallows {@link java.lang.invoke.VarHandle} and {@link sun.misc.Unsafe}, + * which will prevent us from selecting our {@code UnsafeAtomicHelper} strategy. */ @SuppressWarnings({"SunApi", "removal"}) // b/345822163 private static final ClassLoader NO_UNSAFE = - getClassLoader(ImmutableSet.of(Unsafe.class.getName())); + getClassLoader(ImmutableSet.of("java.lang.invoke.VarHandle", Unsafe.class.getName())); /** - * This classloader disallows {@link sun.misc.Unsafe} and {@link AtomicReferenceFieldUpdater}, - * which will prevent us from selecting our {@code SafeAtomicHelper} strategy. + * This classloader disallows {@link java.lang.invoke.VarHandle}, {@link sun.misc.Unsafe} and + * {@link AtomicReferenceFieldUpdater}, which will prevent us from selecting our {@code + * AtomicReferenceFieldUpdaterAtomicHelper} strategy. */ @SuppressWarnings({"SunApi", "removal"}) // b/345822163 private static final ClassLoader NO_ATOMIC_REFERENCE_FIELD_UPDATER = getClassLoader( - ImmutableSet.of(Unsafe.class.getName(), AtomicReferenceFieldUpdater.class.getName())); + ImmutableSet.of( + "java.lang.invoke.VarHandle", + Unsafe.class.getName(), + AtomicReferenceFieldUpdater.class.getName())); public static TestSuite suite() { // we create a test suite containing a test for every AbstractFutureTest test method and we @@ -84,13 +92,25 @@ public static TestSuite suite() { @Override public void runTest() throws Exception { // First ensure that our classloaders are initializing the correct helper versions - checkHelperVersion(getClass().getClassLoader(), "UnsafeAtomicHelper"); + if (isJava8()) { + checkHelperVersion(getClass().getClassLoader(), "UnsafeAtomicHelper"); + } else { + checkHelperVersion(getClass().getClassLoader(), "VarHandleAtomicHelper"); + } + checkHelperVersion(NO_VAR_HANDLE, "UnsafeAtomicHelper"); checkHelperVersion(NO_UNSAFE, "AtomicReferenceFieldUpdaterAtomicHelper"); checkHelperVersion(NO_ATOMIC_REFERENCE_FIELD_UPDATER, "SynchronizedHelper"); // Run the corresponding AbstractFutureTest test method in a new classloader that disallows // certain core jdk classes. ClassLoader oldClassLoader = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader(NO_UNSAFE); + try { + runTestMethod(NO_VAR_HANDLE); + } finally { + Thread.currentThread().setContextClassLoader(oldClassLoader); + } + Thread.currentThread().setContextClassLoader(NO_UNSAFE); try { runTestMethod(NO_UNSAFE); @@ -142,4 +162,8 @@ public Class loadClass(String name) throws ClassNotFoundException { } }; } + + private static boolean isJava8() { + return JAVA_SPECIFICATION_VERSION.value().equals("1.8"); + } } diff --git a/guava/src/com/google/common/util/concurrent/AbstractFuture.java b/guava/src/com/google/common/util/concurrent/AbstractFuture.java index d57d586fda7f..f12f33d2a467 100644 --- a/guava/src/com/google/common/util/concurrent/AbstractFuture.java +++ b/guava/src/com/google/common/util/concurrent/AbstractFuture.java @@ -29,8 +29,11 @@ import com.google.common.util.concurrent.internal.InternalFutures; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.ForOverride; +import com.google.j2objc.annotations.J2ObjCIncompatible; import com.google.j2objc.annotations.ReflectionSupport; import com.google.j2objc.annotations.RetainedLocalRef; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; import java.lang.reflect.Field; import java.security.AccessController; import java.security.PrivilegedActionException; @@ -152,35 +155,60 @@ public final boolean cancel(boolean mayInterruptIfRunning) { private static final AtomicHelper ATOMIC_HELPER; + /** + * Returns the result of calling {@link MethodHandles#lookup} from inside {@link AbstractFuture}. + * By virtue of being created there, it has access to the private fields of {@link + * AbstractFuture}, so that access is available to anyone who calls this method—specifically, to + * {@link VarHandleAtomicHelper}. + * + *
This "shouldn't" be necessary: {@link VarHandleAtomicHelper} and {@link AbstractFuture} + * "should" be nestmates, so a call to {@link MethodHandles#lookup} inside {@link + * VarHandleAtomicHelper} "should" have access to each other's private fields. However, our + * open-source build uses {@code -source 8 -target 8}, so the class files from that build can't + * express nestmates. Thus, when those class files are used from Java 9 or higher (i.e., high + * enough to trigger the {@link VarHandle} code path), such a lookup would fail with an {@link + * IllegalAccessException}. + * + *
Note that we do not have a similar problem with the fields in {@link Waiter} because those + * fields are not private. (We could solve the problem with {@link AbstractFuture} fields in the + * same way if we wanted.) + */ + private static MethodHandles.Lookup methodHandlesLookupFromWithinAbstractFuture() { + return MethodHandles.lookup(); + } + static { AtomicHelper helper; Throwable thrownUnsafeFailure = null; Throwable thrownAtomicReferenceFieldUpdaterFailure = null; - try { - helper = new UnsafeAtomicHelper(); - } catch (Exception | Error unsafeFailure) { // sneaky checked exception - thrownUnsafeFailure = unsafeFailure; - // catch absolutely everything and fall through to our - // 'AtomicReferenceFieldUpdaterAtomicHelper' The access control checks that ARFU does means - // the caller class has to be AbstractFuture instead of - // AtomicReferenceFieldUpdaterAtomicHelper, so we annoyingly define these here + helper = VarHandleAtomicHelperMaker.INSTANCE.tryMakeVarHandleAtomicHelper(); + if (helper == null) { try { - helper = - new AtomicReferenceFieldUpdaterAtomicHelper( - newUpdater(Waiter.class, Thread.class, "thread"), - newUpdater(Waiter.class, Waiter.class, "next"), - newUpdater(AbstractFuture.class, Waiter.class, "waiters"), - newUpdater(AbstractFuture.class, Listener.class, "listeners"), - newUpdater(AbstractFuture.class, Object.class, "value")); - } catch (Exception // sneaky checked exception - | Error atomicReferenceFieldUpdaterFailure) { - // Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause - // getDeclaredField to throw a NoSuchFieldException when the field is definitely there. - // For these users fallback to a suboptimal implementation, based on synchronized. This will - // be a definite performance hit to those users. - thrownAtomicReferenceFieldUpdaterFailure = atomicReferenceFieldUpdaterFailure; - helper = new SynchronizedHelper(); + helper = new UnsafeAtomicHelper(); + } catch (Exception | Error unsafeFailure) { // sneaky checked exception + thrownUnsafeFailure = unsafeFailure; + // catch absolutely everything and fall through to our + // 'AtomicReferenceFieldUpdaterAtomicHelper' The access control checks that ARFU does means + // the caller class has to be AbstractFuture instead of + // AtomicReferenceFieldUpdaterAtomicHelper, so we annoyingly define these here + try { + helper = + new AtomicReferenceFieldUpdaterAtomicHelper( + newUpdater(Waiter.class, Thread.class, "thread"), + newUpdater(Waiter.class, Waiter.class, "next"), + newUpdater(AbstractFuture.class, Waiter.class, "waiters"), + newUpdater(AbstractFuture.class, Listener.class, "listeners"), + newUpdater(AbstractFuture.class, Object.class, "value")); + } catch (Exception // sneaky checked exception + | Error atomicReferenceFieldUpdaterFailure) { + // Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause + // getDeclaredField to throw a NoSuchFieldException when the field is definitely there. + // For these users fallback to a suboptimal implementation, based on synchronized. This + // will be a definite performance hit to those users. + thrownAtomicReferenceFieldUpdaterFailure = atomicReferenceFieldUpdaterFailure; + helper = new SynchronizedHelper(); + } } } ATOMIC_HELPER = helper; @@ -202,6 +230,42 @@ public final boolean cancel(boolean mayInterruptIfRunning) { } } + private enum VarHandleAtomicHelperMaker { + INSTANCE { + /** + * Implementation used by non-J2ObjC environments (aside, of course, from those that have + * supersource for the entirety of {@link AbstractFuture}). + */ + @Override + @J2ObjCIncompatible + @CheckForNull + AtomicHelper tryMakeVarHandleAtomicHelper() { + try { + /* + * We first use reflection to check whether VarHandle exists. If we instead just tried to + * load our class directly (which would trigger non-reflective loading of VarHandle), then + * an error might be thrown even before we enter the `try` block: + * https://github.com/google/truth/issues/333#issuecomment-765652454 + * + * Also, it's nice that this approach should let us catch *only* ClassNotFoundException + * instead of having to catch more broadly (potentially even including, say, a + * StackOverflowError). + */ + Class.forName("java.lang.invoke.VarHandle"); + return new VarHandleAtomicHelper(); + } catch (ClassNotFoundException beforeJava9) { + return null; + } + } + }; + + /** Implementation used by J2ObjC environments, overridden for other environments. */ + @CheckForNull + AtomicHelper tryMakeVarHandleAtomicHelper() { + return null; + } + } + /** Waiter links form a Treiber stack, in the {@link #waiters} field. */ private static final class Waiter { static final Waiter TOMBSTONE = new Waiter(false /* ignored param */); @@ -1344,6 +1408,68 @@ abstract boolean casListeners( abstract boolean casValue(AbstractFuture future, @CheckForNull Object expect, Object update); } + /** {@link AtomicHelper} based on {@link VarHandle}. */ + @J2ObjCIncompatible + // We use this class only after confirming that VarHandle is available at runtime. + @SuppressWarnings("Java8ApiChecker") + @IgnoreJRERequirement + private static final class VarHandleAtomicHelper extends AtomicHelper { + static final VarHandle waiterThreadUpdater; + static final VarHandle waiterNextUpdater; + static final VarHandle waitersUpdater; + static final VarHandle listenersUpdater; + static final VarHandle valueUpdater; + + static { + MethodHandles.Lookup lookup = methodHandlesLookupFromWithinAbstractFuture(); + try { + waiterThreadUpdater = lookup.findVarHandle(Waiter.class, "thread", Thread.class); + waiterNextUpdater = lookup.findVarHandle(Waiter.class, "next", Waiter.class); + waitersUpdater = lookup.findVarHandle(AbstractFuture.class, "waiters", Waiter.class); + listenersUpdater = lookup.findVarHandle(AbstractFuture.class, "listeners", Listener.class); + valueUpdater = lookup.findVarHandle(AbstractFuture.class, "value", Object.class); + } catch (ReflectiveOperationException e) { + throw new ExceptionInInitializerError(e); + } + } + + @Override + void putThread(Waiter waiter, Thread newValue) { + waiterThreadUpdater.setRelease(waiter, newValue); + } + + @Override + void putNext(Waiter waiter, @CheckForNull Waiter newValue) { + waiterNextUpdater.setRelease(waiter, newValue); + } + + @Override + boolean casWaiters( + AbstractFuture future, @CheckForNull Waiter expect, @CheckForNull Waiter update) { + return waitersUpdater.compareAndSet(future, expect, update); + } + + @Override + boolean casListeners(AbstractFuture future, @CheckForNull Listener expect, Listener update) { + return listenersUpdater.compareAndSet(future, expect, update); + } + + @Override + Listener gasListeners(AbstractFuture future, Listener update) { + return (Listener) listenersUpdater.getAndSet(future, update); + } + + @Override + Waiter gasWaiters(AbstractFuture future, Waiter update) { + return (Waiter) waitersUpdater.getAndSet(future, update); + } + + @Override + boolean casValue(AbstractFuture future, @CheckForNull Object expect, Object update) { + return valueUpdater.compareAndSet(future, expect, update); + } + } + /** * {@link AtomicHelper} based on {@link sun.misc.Unsafe}. *