Make AbstractFuture use VarHandle when available.

android/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 @@
  * <p>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.
- *
- * <ul>
- *   <li>SafeAtomicHelper: uses AtomicReferenceFieldsUpdaters to implement synchronization
- *   <li>SynchronizedHelper: uses {@code synchronized} blocks for synchronization
- * </ul>
+ * introduces some testing difficulties. This test exercises the fallback strategies.
  *
- * 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.
+ * <p>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");
+  }
 }

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 @@
  * <p>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.
- *
- * <ul>
- *   <li>SafeAtomicHelper: uses AtomicReferenceFieldsUpdaters to implement synchronization
- *   <li>SynchronizedHelper: uses {@code synchronized} blocks for synchronization
- * </ul>
+ * introduces some testing difficulties. This test exercises the fallback strategies.
  *
- * 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.
+ * <p>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");
+  }
 }

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}.
+   *
+   * <p>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}.
+   *
+   * <p>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) from
+           * within a `try` block, 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");
+        } catch (ClassNotFoundException beforeJava9) {
+          return null;
+        }
+        return new VarHandleAtomicHelper();
+      }
+    };
+
+    /** 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,73 @@ 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) {
+        // Those fields exist.
+        throw newLinkageError(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);
+    }
+
+    private static LinkageError newLinkageError(Throwable cause) {
+      return new LinkageError(cause.toString(), cause);
+    }
+  }
+
   /**
    * {@link AtomicHelper} based on {@link sun.misc.Unsafe}.
    *