Skip to content

Commit b4da629

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Introduce an AtomicHelper test simple enough to run under Android.
This test doesn't exercise any alternative `AtomicHelper` implementations; it merely checks that `AbstractFutureState` selects the implementation that we expect under each given environment. This CL is prompted by my discovery today that I'd gotten a Proguard config wrong. I corrected the config in cl/742724817, but this new test suggests that (at least in under our monorepo's build toolchain, which differs from typical external Android toolchains!) `AbstractFutureState` continued doing the right thing throughout, even under optimization/minification (which I tested locally), apparently because the optimizer autodetected that we were reflecting on the fields. Still, we shouldn't rely on that. I also cleaned up the existing `AtomicHelper` tests to use a package-private method instead of reading a `private` field. The motivation for that change is that my new test was failing under minification because the class name did not match the expected value. That forced me to expose something more stable about the choice of `AtomicHelper` to the tests. And once I had that, I figured that I might as well use it in the existing tests. Maybe someday I'll update `AggregateFutureStateFallbackAtomicHelperTest` similarly. This CL continues [work to migrate `guava-android` off `Unsafe`](#7742). RELNOTES=n/a PiperOrigin-RevId: 742859752
1 parent 9159c2f commit b4da629

File tree

6 files changed

+141
-8
lines changed

6 files changed

+141
-8
lines changed
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (C) 2015 The Guava Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
15+
package com.google.common.util.concurrent;
16+
17+
import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
18+
import static com.google.common.truth.Truth.assertThat;
19+
20+
import junit.framework.TestCase;
21+
import org.jspecify.annotations.NullUnmarked;
22+
23+
/**
24+
* Tests that {@link AbstractFutureState} uses the expected {@code AtomicHelper} implementation.
25+
*
26+
* <p>We have more thorough testing of {@code AtomicHelper} implementations in {@link
27+
* AbstractFutureFallbackAtomicHelperTest}. The advantage to this test is that it can run under
28+
* Android.
29+
*/
30+
@NullUnmarked
31+
public class AbstractFutureDefaultAtomicHelperTest extends TestCase {
32+
public void testUsingExpectedAtomicHelper() throws Exception {
33+
assertThat(AbstractFutureState.atomicHelperTypeForTest()).isEqualTo("UnsafeAtomicHelper");
34+
}
35+
36+
private static boolean isJava8() {
37+
return JAVA_SPECIFICATION_VERSION.value().equals("1.8");
38+
}
39+
}

android/guava-tests/test/com/google/common/util/concurrent/AbstractFutureFallbackAtomicHelperTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515
package com.google.common.util.concurrent;
1616

1717
import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
18+
import static com.google.common.truth.Truth.assertThat;
1819

1920
import com.google.common.collect.ImmutableSet;
20-
import java.lang.reflect.Field;
2121
import java.lang.reflect.Method;
2222
import java.lang.reflect.Modifier;
2323
import java.net.URLClassLoader;
@@ -111,9 +111,9 @@ private void checkHelperVersion(ClassLoader classLoader, String expectedHelperCl
111111
throws Exception {
112112
// Make sure we are actually running with the expected helper implementation
113113
Class<?> abstractFutureStateClass = classLoader.loadClass(AbstractFutureState.class.getName());
114-
Field helperField = abstractFutureStateClass.getDeclaredField("ATOMIC_HELPER");
115-
helperField.setAccessible(true);
116-
assertEquals(expectedHelperClassName, helperField.get(null).getClass().getSimpleName());
114+
Method helperMethod = abstractFutureStateClass.getDeclaredMethod("atomicHelperTypeForTest");
115+
helperMethod.setAccessible(true);
116+
assertThat(helperMethod.invoke(null)).isEqualTo(expectedHelperClassName);
117117
}
118118

119119
private static ClassLoader getClassLoader(final Set<String> disallowedClassNames) {

android/guava/src/com/google/common/util/concurrent/AbstractFutureState.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static java.util.logging.Level.SEVERE;
2525

2626
import com.google.common.annotations.GwtCompatible;
27+
import com.google.common.annotations.VisibleForTesting;
2728
import com.google.common.util.concurrent.AbstractFuture.Listener;
2829
import com.google.common.util.concurrent.internal.InternalFutureFailureAccess;
2930
import com.google.j2objc.annotations.ReflectionSupport;
@@ -490,6 +491,11 @@ private void removeWaiter(Waiter node) {
490491
// blocking. This value is what AbstractQueuedSynchronizer uses.
491492
private static final long SPIN_THRESHOLD_NANOS = 1000L;
492493

494+
@VisibleForTesting
495+
static String atomicHelperTypeForTest() {
496+
return ATOMIC_HELPER.atomicHelperTypeForTest();
497+
}
498+
493499
private abstract static class AtomicHelper {
494500
/** Non-volatile write of the thread to the {@link Waiter#thread} field. */
495501
abstract void putThread(Waiter waiter, Thread newValue);
@@ -514,6 +520,8 @@ abstract boolean casListeners(
514520
/** Performs a CAS operation on {@link AbstractFutureState#valueField}. */
515521
abstract boolean casValue(
516522
AbstractFutureState<?> future, @Nullable Object expect, Object update);
523+
524+
abstract String atomicHelperTypeForTest();
517525
}
518526

519527
/**
@@ -622,6 +630,11 @@ boolean casListeners(
622630
boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object update) {
623631
return UNSAFE.compareAndSwapObject(future, VALUE_OFFSET, expect, update);
624632
}
633+
634+
@Override
635+
String atomicHelperTypeForTest() {
636+
return "UnsafeAtomicHelper";
637+
}
625638
}
626639

627640
/** {@link AtomicHelper} based on {@link AtomicReferenceFieldUpdater}. */
@@ -678,6 +691,11 @@ boolean casListeners(
678691
boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object update) {
679692
return valueUpdater.compareAndSet(future, expect, update);
680693
}
694+
695+
@Override
696+
String atomicHelperTypeForTest() {
697+
return "AtomicReferenceFieldUpdaterAtomicHelper";
698+
}
681699
}
682700

683701
/**
@@ -753,5 +771,10 @@ boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object
753771
return false;
754772
}
755773
}
774+
775+
@Override
776+
String atomicHelperTypeForTest() {
777+
return "SynchronizedHelper";
778+
}
756779
}
757780
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Copyright (C) 2015 The Guava Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
15+
package com.google.common.util.concurrent;
16+
17+
import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
18+
import static com.google.common.truth.Truth.assertThat;
19+
20+
import junit.framework.TestCase;
21+
import org.jspecify.annotations.NullUnmarked;
22+
23+
/**
24+
* Tests that {@link AbstractFutureState} uses the expected {@code AtomicHelper} implementation.
25+
*
26+
* <p>We have more thorough testing of {@code AtomicHelper} implementations in {@link
27+
* AbstractFutureFallbackAtomicHelperTest}. The advantage to this test is that it can run under
28+
* Android.
29+
*/
30+
@NullUnmarked
31+
public class AbstractFutureDefaultAtomicHelperTest extends TestCase {
32+
public void testUsingExpectedAtomicHelper() throws Exception {
33+
if (isJava8()) {
34+
assertThat(AbstractFutureState.atomicHelperTypeForTest()).isEqualTo("UnsafeAtomicHelper");
35+
} else {
36+
assertThat(AbstractFutureState.atomicHelperTypeForTest()).isEqualTo("VarHandleAtomicHelper");
37+
}
38+
}
39+
40+
private static boolean isJava8() {
41+
return JAVA_SPECIFICATION_VERSION.value().equals("1.8");
42+
}
43+
}

guava-tests/test/com/google/common/util/concurrent/AbstractFutureFallbackAtomicHelperTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515
package com.google.common.util.concurrent;
1616

1717
import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
18+
import static com.google.common.truth.Truth.assertThat;
1819

1920
import com.google.common.collect.ImmutableSet;
20-
import java.lang.reflect.Field;
2121
import java.lang.reflect.Method;
2222
import java.lang.reflect.Modifier;
2323
import java.net.URLClassLoader;
@@ -134,9 +134,9 @@ private void checkHelperVersion(ClassLoader classLoader, String expectedHelperCl
134134
throws Exception {
135135
// Make sure we are actually running with the expected helper implementation
136136
Class<?> abstractFutureStateClass = classLoader.loadClass(AbstractFutureState.class.getName());
137-
Field helperField = abstractFutureStateClass.getDeclaredField("ATOMIC_HELPER");
138-
helperField.setAccessible(true);
139-
assertEquals(expectedHelperClassName, helperField.get(null).getClass().getSimpleName());
137+
Method helperMethod = abstractFutureStateClass.getDeclaredMethod("atomicHelperTypeForTest");
138+
helperMethod.setAccessible(true);
139+
assertThat(helperMethod.invoke(null)).isEqualTo(expectedHelperClassName);
140140
}
141141

142142
private static ClassLoader getClassLoader(final Set<String> disallowedClassNames) {

guava/src/com/google/common/util/concurrent/AbstractFutureState.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static java.util.logging.Level.SEVERE;
2525

2626
import com.google.common.annotations.GwtCompatible;
27+
import com.google.common.annotations.VisibleForTesting;
2728
import com.google.common.util.concurrent.AbstractFuture.Listener;
2829
import com.google.common.util.concurrent.internal.InternalFutureFailureAccess;
2930
import com.google.j2objc.annotations.J2ObjCIncompatible;
@@ -496,6 +497,11 @@ private void removeWaiter(Waiter node) {
496497
// blocking. This value is what AbstractQueuedSynchronizer uses.
497498
private static final long SPIN_THRESHOLD_NANOS = 1000L;
498499

500+
@VisibleForTesting
501+
static String atomicHelperTypeForTest() {
502+
return ATOMIC_HELPER.atomicHelperTypeForTest();
503+
}
504+
499505
private enum VarHandleAtomicHelperMaker {
500506
INSTANCE {
501507
/**
@@ -554,6 +560,8 @@ abstract boolean casListeners(
554560
/** Performs a CAS operation on {@link AbstractFutureState#valueField}. */
555561
abstract boolean casValue(
556562
AbstractFutureState<?> future, @Nullable Object expect, Object update);
563+
564+
abstract String atomicHelperTypeForTest();
557565
}
558566

559567
/** {@link AtomicHelper} based on {@link VarHandle}. */
@@ -624,6 +632,11 @@ boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object
624632
private static LinkageError newLinkageError(Throwable cause) {
625633
return new LinkageError(cause.toString(), cause);
626634
}
635+
636+
@Override
637+
String atomicHelperTypeForTest() {
638+
return "VarHandleAtomicHelper";
639+
}
627640
}
628641

629642
/**
@@ -716,6 +729,11 @@ boolean casListeners(
716729
boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object update) {
717730
return UNSAFE.compareAndSwapObject(future, VALUE_OFFSET, expect, update);
718731
}
732+
733+
@Override
734+
String atomicHelperTypeForTest() {
735+
return "UnsafeAtomicHelper";
736+
}
719737
}
720738

721739
/** {@link AtomicHelper} based on {@link AtomicReferenceFieldUpdater}. */
@@ -772,6 +790,11 @@ boolean casListeners(
772790
boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object update) {
773791
return valueUpdater.compareAndSet(future, expect, update);
774792
}
793+
794+
@Override
795+
String atomicHelperTypeForTest() {
796+
return "AtomicReferenceFieldUpdaterAtomicHelper";
797+
}
775798
}
776799

777800
/**
@@ -847,5 +870,10 @@ boolean casValue(AbstractFutureState<?> future, @Nullable Object expect, Object
847870
return false;
848871
}
849872
}
873+
874+
@Override
875+
String atomicHelperTypeForTest() {
876+
return "SynchronizedHelper";
877+
}
850878
}
851879
}

0 commit comments

Comments
 (0)