Skip to content

Commit c9c543e

Browse files
cpovirkGoogle Java Core Libraries
authored and
Google Java Core Libraries
committed
Avoid reflection on java.util.concurrent internals during tests.
Under modern JDKs, it will fail. This lets us remove one of our `--add-opens` lines. (I haven't looked into [the others](#5801 (comment)).) Example failure: ``` java.lang.reflect.InaccessibleObjectException: Unable to make protected final java.lang.Thread java.util.concurrent.locks.AbstractOwnableSynchronizer.getExclusiveOwnerThread() accessible: module java.base does not "opens java.util.concurrent.locks" to unnamed module @5ba184fc; did you mean --add-opens=java.base/java.util.concurrent.locks=ALL-UNNAMED at java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:348) at java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:280) at java.lang.reflect.Method.checkCanSetAccessible(Method.java:198) at java.lang.reflect.Method.setAccessible(Method.java:192) at com.google.common.util.concurrent.InterruptibleTaskTest.testInterruptIsSlow(InterruptibleTaskTest.java:160) ``` Relevant to #5801 RELNOTES=n/a PiperOrigin-RevId: 469515689
1 parent 101dc3e commit c9c543e

File tree

6 files changed

+18
-20
lines changed

6 files changed

+18
-20
lines changed

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

+3-8
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,10 @@
1717

1818
import static com.google.common.truth.Truth.assertThat;
1919

20-
import java.lang.reflect.Method;
20+
import com.google.common.util.concurrent.InterruptibleTask.Blocker;
2121
import java.nio.channels.spi.AbstractInterruptibleChannel;
2222
import java.util.concurrent.CountDownLatch;
2323
import java.util.concurrent.TimeUnit;
24-
import java.util.concurrent.locks.AbstractOwnableSynchronizer;
2524
import java.util.concurrent.locks.LockSupport;
2625
import junit.framework.TestCase;
2726

@@ -152,12 +151,8 @@ public void run() {
152151
// waiting for the slow interrupting thread to complete Thread.interrupt
153152
awaitBlockedOnInstanceOf(runner, InterruptibleTask.Blocker.class);
154153

155-
Object blocker = LockSupport.getBlocker(runner);
156-
assertThat(blocker).isInstanceOf(AbstractOwnableSynchronizer.class);
157-
Method getExclusiveOwnerThread =
158-
AbstractOwnableSynchronizer.class.getDeclaredMethod("getExclusiveOwnerThread");
159-
getExclusiveOwnerThread.setAccessible(true);
160-
Thread owner = (Thread) getExclusiveOwnerThread.invoke(blocker);
154+
Blocker blocker = (Blocker) LockSupport.getBlocker(runner);
155+
Thread owner = blocker.getOwner();
161156
assertThat(owner).isSameInstanceAs(interrupter);
162157

163158
slowChannel.exitClose.countDown(); // release the interrupter

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

+5
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,11 @@ private void setOwner(Thread thread) {
233233
super.setExclusiveOwnerThread(thread);
234234
}
235235

236+
@VisibleForTesting
237+
Thread getOwner() {
238+
return super.getExclusiveOwnerThread();
239+
}
240+
236241
@Override
237242
public String toString() {
238243
return task.toString();

android/pom.xml

+1-2
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@
425425
</activation>
426426
<properties>
427427
<!--
428-
Some tests need reflective access to the internals of these packages. It is ony the
428+
Some tests need reflective access to the internals of these packages. It is only the
429429
tests themselves and not the code being tested that needs that access, though there's no
430430
obvious way to ensure that.
431431
@@ -435,7 +435,6 @@
435435
<test.add.opens>
436436
--add-opens java.base/java.lang=ALL-UNNAMED
437437
--add-opens java.base/java.util=ALL-UNNAMED
438-
--add-opens java.base/java.util.concurrent.locks=ALL-UNNAMED
439438
--add-opens java.base/sun.security.jca=ALL-UNNAMED
440439
</test.add.opens>
441440
</properties>

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

+3-8
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,10 @@
1717

1818
import static com.google.common.truth.Truth.assertThat;
1919

20-
import java.lang.reflect.Method;
20+
import com.google.common.util.concurrent.InterruptibleTask.Blocker;
2121
import java.nio.channels.spi.AbstractInterruptibleChannel;
2222
import java.util.concurrent.CountDownLatch;
2323
import java.util.concurrent.TimeUnit;
24-
import java.util.concurrent.locks.AbstractOwnableSynchronizer;
2524
import java.util.concurrent.locks.LockSupport;
2625
import junit.framework.TestCase;
2726

@@ -152,12 +151,8 @@ public void run() {
152151
// waiting for the slow interrupting thread to complete Thread.interrupt
153152
awaitBlockedOnInstanceOf(runner, InterruptibleTask.Blocker.class);
154153

155-
Object blocker = LockSupport.getBlocker(runner);
156-
assertThat(blocker).isInstanceOf(AbstractOwnableSynchronizer.class);
157-
Method getExclusiveOwnerThread =
158-
AbstractOwnableSynchronizer.class.getDeclaredMethod("getExclusiveOwnerThread");
159-
getExclusiveOwnerThread.setAccessible(true);
160-
Thread owner = (Thread) getExclusiveOwnerThread.invoke(blocker);
154+
Blocker blocker = (Blocker) LockSupport.getBlocker(runner);
155+
Thread owner = blocker.getOwner();
161156
assertThat(owner).isSameInstanceAs(interrupter);
162157

163158
slowChannel.exitClose.countDown(); // release the interrupter

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

+5
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,11 @@ private void setOwner(Thread thread) {
233233
super.setExclusiveOwnerThread(thread);
234234
}
235235

236+
@VisibleForTesting
237+
Thread getOwner() {
238+
return super.getExclusiveOwnerThread();
239+
}
240+
236241
@Override
237242
public String toString() {
238243
return task.toString();

pom.xml

+1-2
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@
432432
</activation>
433433
<properties>
434434
<!--
435-
Some tests need reflective access to the internals of these packages. It is ony the
435+
Some tests need reflective access to the internals of these packages. It is only the
436436
tests themselves and not the code being tested that needs that access, though there's no
437437
obvious way to ensure that.
438438
@@ -442,7 +442,6 @@
442442
<test.add.opens>
443443
--add-opens java.base/java.lang=ALL-UNNAMED
444444
--add-opens java.base/java.util=ALL-UNNAMED
445-
--add-opens java.base/java.util.concurrent.locks=ALL-UNNAMED
446445
--add-opens java.base/sun.security.jca=ALL-UNNAMED
447446
</test.add.opens>
448447
</properties>

0 commit comments

Comments
 (0)