Skip to content

Commit

Permalink
Avoid reflection on java.util.concurrent internals during tests.
Browse files Browse the repository at this point in the history
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: 469500716
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Aug 23, 2022
1 parent 101dc3e commit 9962753
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@

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

import java.lang.reflect.Method;
import com.google.common.util.concurrent.InterruptibleTask.Blocker;
import java.nio.channels.spi.AbstractInterruptibleChannel;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.AbstractOwnableSynchronizer;
import java.util.concurrent.locks.LockSupport;
import junit.framework.TestCase;

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

Object blocker = LockSupport.getBlocker(runner);
assertThat(blocker).isInstanceOf(AbstractOwnableSynchronizer.class);
Method getExclusiveOwnerThread =
AbstractOwnableSynchronizer.class.getDeclaredMethod("getExclusiveOwnerThread");
getExclusiveOwnerThread.setAccessible(true);
Thread owner = (Thread) getExclusiveOwnerThread.invoke(blocker);
Blocker blocker = (Blocker) LockSupport.getBlocker(runner);
Thread owner = blocker.getOwner();
assertThat(owner).isSameInstanceAs(interrupter);

slowChannel.exitClose.countDown(); // release the interrupter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,11 @@ private void setOwner(Thread thread) {
super.setExclusiveOwnerThread(thread);
}

@VisibleForTesting
Thread getOwner() {
return super.getExclusiveOwnerThread();
}

@Override
public String toString() {
return task.toString();
Expand Down
3 changes: 1 addition & 2 deletions android/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@
</activation>
<properties>
<!--
Some tests need reflective access to the internals of these packages. It is ony the
Some tests need reflective access to the internals of these packages. It is only the
tests themselves and not the code being tested that needs that access, though there's no
obvious way to ensure that.
Expand All @@ -435,7 +435,6 @@
<test.add.opens>
--add-opens java.base/java.lang=ALL-UNNAMED
--add-opens java.base/java.util=ALL-UNNAMED
--add-opens java.base/java.util.concurrent.locks=ALL-UNNAMED
--add-opens java.base/sun.security.jca=ALL-UNNAMED
</test.add.opens>
</properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@

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

import java.lang.reflect.Method;
import com.google.common.util.concurrent.InterruptibleTask.Blocker;
import java.nio.channels.spi.AbstractInterruptibleChannel;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.AbstractOwnableSynchronizer;
import java.util.concurrent.locks.LockSupport;
import junit.framework.TestCase;

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

Object blocker = LockSupport.getBlocker(runner);
assertThat(blocker).isInstanceOf(AbstractOwnableSynchronizer.class);
Method getExclusiveOwnerThread =
AbstractOwnableSynchronizer.class.getDeclaredMethod("getExclusiveOwnerThread");
getExclusiveOwnerThread.setAccessible(true);
Thread owner = (Thread) getExclusiveOwnerThread.invoke(blocker);
Blocker blocker = (Blocker) LockSupport.getBlocker(runner);
Thread owner = blocker.getOwner();
assertThat(owner).isSameInstanceAs(interrupter);

slowChannel.exitClose.countDown(); // release the interrupter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,11 @@ private void setOwner(Thread thread) {
super.setExclusiveOwnerThread(thread);
}

@VisibleForTesting
Thread getOwner() {
return super.getExclusiveOwnerThread();
}

@Override
public String toString() {
return task.toString();
Expand Down
3 changes: 1 addition & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@
</activation>
<properties>
<!--
Some tests need reflective access to the internals of these packages. It is ony the
Some tests need reflective access to the internals of these packages. It is only the
tests themselves and not the code being tested that needs that access, though there's no
obvious way to ensure that.
Expand All @@ -442,7 +442,6 @@
<test.add.opens>
--add-opens java.base/java.lang=ALL-UNNAMED
--add-opens java.base/java.util=ALL-UNNAMED
--add-opens java.base/java.util.concurrent.locks=ALL-UNNAMED
--add-opens java.base/sun.security.jca=ALL-UNNAMED
</test.add.opens>
</properties>
Expand Down

0 comments on commit 9962753

Please sign in to comment.