Skip to content

Commit ae5e4d6

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Call Thread.stop reflectively, and fix MonitorTestCase to fail in case of an uncaughtThrowable.
Under Java 26, `Thread.stop` is disappearing entirely. We've already seen that it began throwing `UnsupportedOperationException` in Java 20. But for testing under older versions of Java in our open-source repo, it's potentially still useful. (It's probably _not_ useful for Android tests, even [under Marshmallow](https://cs.android.com/android/platform/superproject/+/android-6.0.1_r81:libcore/libart/src/main/java/java/lang/Thread.java;l=1077-1079,1087-1089;drc=3a2414f45502454bdf0a4ff28f50682f72e4f435).) (Compare 388e098 for `Thread.suspend` and `resume`.) While dealing with that, I thought more about the fact that `TestThread.tearDown` has been throwing `UnsupportedOperationException` under recent Java versions. That means that it doesn't continue on to try to `join()` a thread that might hang forever (good), but it also means that it never gets to check for `uncaughtThrowable` (bad). I addressed that by putting `stop()` and `join()` in a `try`-`catch`. But even beyond that, once `TestThread` _does_ check for `uncaughtThrowable` and throw when it finds one, the `AssertionError` would still be being ignored, just as the `UnsupportedOperationException` from `stop()` was being ignored. That happens because `MonitorTestCase` passes `true` for the `suppressThrows` parameter of `TearDownStack`. Oops. I've removed `true` so that we now get [the default behavior, `false`](https://github.com/google/guava/blob/9a179fc66b79c47e5f5d0c59bceb7162e5d2abe2/guava-testlib/src/com/google/common/testing/TearDownStack.java#L53). That would have led to failures back when we were letting the exception from `stop()` propagate, but now it is safe. Thankfully, it doesn't reveal any previously hidden cases in which we had an `uncaughtThrowable`. RELNOTES=n/a PiperOrigin-RevId: 812875341
1 parent 3c0e704 commit ae5e4d6

File tree

4 files changed

+38
-16
lines changed

4 files changed

+38
-16
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public void setSatisfied(boolean satisfied) {
5050

5151
private final boolean interruptible;
5252
private Monitor monitor;
53-
private final TearDownStack tearDownStack = new TearDownStack(true);
53+
private final TearDownStack tearDownStack = new TearDownStack();
5454
private TestThread<Monitor> thread1;
5555
private TestThread<Monitor> thread2;
5656

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,26 @@ public TestThread(L lockLikeObject, String threadName) {
6868
start();
6969
}
7070

71-
// Thread.stop() is okay because all threads started by a test are dying at the end of the test,
72-
// so there is no object state put at risk by stopping the threads abruptly. In some cases a test
73-
// may put a thread into an uninterruptible operation intentionally, so there is no other way to
74-
// clean up these threads.
75-
@SuppressWarnings("deprecation")
71+
/*
72+
* TODO: b/318391980 - Once we test only under Java 20 and higher, avoid calling Thread.stop. As
73+
* of Java 20, it always throws an exception, and as of Java 26, the method does not even exist.
74+
* For now, we continue using it to clean up under older JDKs.
75+
*
76+
* Our usages should at least be *relatively* safe: Typically, threads started by a test are dying
77+
* at the end of the test, so there is no object state put at risk by stopping the threads
78+
* abruptly. In other cases, a test may put a thread into an uninterruptible operation
79+
* intentionally, so there is no other way to clean up these threads. (The better solution,
80+
* though, would be to run the tests that use TestThread in separate VMs so that their threads
81+
* don't hang around during other tests.)
82+
*/
7683
@Override
7784
public void tearDown() throws Exception {
78-
stop();
79-
join();
85+
try {
86+
Thread.class.getMethod("stop").invoke(this);
87+
join();
88+
} catch (ReflectiveOperationException e) {
89+
// stop() threw or did not exist. Don't join() the thread, which might hang forever.
90+
}
8091

8192
if (uncaughtThrowable != null) {
8293
throw new AssertionError("Uncaught throwable in " + getName(), uncaughtThrowable);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public void setSatisfied(boolean satisfied) {
5050

5151
private final boolean interruptible;
5252
private Monitor monitor;
53-
private final TearDownStack tearDownStack = new TearDownStack(true);
53+
private final TearDownStack tearDownStack = new TearDownStack();
5454
private TestThread<Monitor> thread1;
5555
private TestThread<Monitor> thread2;
5656

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,26 @@ public TestThread(L lockLikeObject, String threadName) {
6868
start();
6969
}
7070

71-
// Thread.stop() is okay because all threads started by a test are dying at the end of the test,
72-
// so there is no object state put at risk by stopping the threads abruptly. In some cases a test
73-
// may put a thread into an uninterruptible operation intentionally, so there is no other way to
74-
// clean up these threads.
75-
@SuppressWarnings("deprecation")
71+
/*
72+
* TODO: b/318391980 - Once we test only under Java 20 and higher, avoid calling Thread.stop. As
73+
* of Java 20, it always throws an exception, and as of Java 26, the method does not even exist.
74+
* For now, we continue using it to clean up under older JDKs.
75+
*
76+
* Our usages should at least be *relatively* safe: Typically, threads started by a test are dying
77+
* at the end of the test, so there is no object state put at risk by stopping the threads
78+
* abruptly. In other cases, a test may put a thread into an uninterruptible operation
79+
* intentionally, so there is no other way to clean up these threads. (The better solution,
80+
* though, would be to run the tests that use TestThread in separate VMs so that their threads
81+
* don't hang around during other tests.)
82+
*/
7683
@Override
7784
public void tearDown() throws Exception {
78-
stop();
79-
join();
85+
try {
86+
Thread.class.getMethod("stop").invoke(this);
87+
join();
88+
} catch (ReflectiveOperationException e) {
89+
// stop() threw or did not exist. Don't join() the thread, which might hang forever.
90+
}
8091

8192
if (uncaughtThrowable != null) {
8293
throw new AssertionError("Uncaught throwable in " + getName(), uncaughtThrowable);

0 commit comments

Comments
 (0)