8320532: Remove Thread/ThreadGroup suspend/resume

Reviewed-by: dholmes, jpai, sspitsyn, smarks
This commit is contained in:
Alan Bateman 2023-12-08 07:10:20 +00:00
parent cb7e3d263a
commit af5c49226c
8 changed files with 36 additions and 350 deletions
src
java.base/share/classes/java/lang
java.management/share/classes/java/lang/management
test/jdk/java
lang
nio/channels/SocketChannel

@ -1647,7 +1647,7 @@ public class Thread implements Runnable {
* interrupt the wait.
* For more information, see
* <a href="{@docRoot}/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html">Why
* are Thread.stop, Thread.suspend and Thread.resume Deprecated?</a>.
* is Thread.stop deprecated and the ability to stop a thread removed?</a>.
*/
@Deprecated(since="1.2", forRemoval=true)
public final void stop() {
@ -1788,44 +1788,6 @@ public class Thread implements Runnable {
return eetop != 0;
}
/**
* Throws {@code UnsupportedOperationException}.
*
* @throws UnsupportedOperationException always
*
* @deprecated This method was originally specified to suspend a thread.
* It was inherently deadlock-prone. If the target thread held a lock on
* a monitor protecting a critical system resource when it was suspended,
* no thread could access the resource until the target thread was resumed.
* If the thread intending to resume the target thread attempted to lock
* the monitor prior to calling {@code resume}, deadlock would result.
* Such deadlocks typically manifested themselves as "frozen" processes.
* For more information, see
* <a href="{@docRoot}/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html">Why
* are Thread.stop, Thread.suspend and Thread.resume Deprecated?</a>.
*/
@Deprecated(since="1.2", forRemoval=true)
public final void suspend() {
throw new UnsupportedOperationException();
}
/**
* Throws {@code UnsupportedOperationException}.
*
* @throws UnsupportedOperationException always
*
* @deprecated This method was originally specified to resume a thread
* suspended with {@link #suspend()}. Suspending a thread was
* inherently deadlock-prone.
* For more information, see
* <a href="{@docRoot}/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html">Why
* are Thread.stop, Thread.suspend and Thread.resume Deprecated?</a>.
*/
@Deprecated(since="1.2", forRemoval=true)
public final void resume() {
throw new UnsupportedOperationException();
}
/**
* Changes the priority of this thread.
*

@ -1,5 +1,5 @@
/*
* Copyright (c) 1995, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1995, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -587,28 +587,6 @@ public class ThreadGroup implements Thread.UncaughtExceptionHandler {
}
}
/**
* Throws {@code UnsupportedOperationException}.
*
* @deprecated This method was originally specified to suspend all threads
* in the thread group.
*/
@Deprecated(since="1.2", forRemoval=true)
public final void suspend() {
throw new UnsupportedOperationException();
}
/**
* Throws {@code UnsupportedOperationException}.
*
* @deprecated This method was originally specified to resume all threads
* in the thread group.
*/
@Deprecated(since="1.2", forRemoval=true)
public final void resume() {
throw new UnsupportedOperationException();
}
/**
* Does nothing.
*

@ -1,6 +1,6 @@
<!doctype html>
<!--
Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
This code is free software; you can redistribute it and/or modify it
@ -158,173 +158,5 @@ wouldn't respond to <code>Thread.stop</code> either.</em> Such
cases include deliberate denial-of-service attacks, and I/O
operations for which thread.stop and thread.interrupt do not work
properly.</p>
<hr>
<h2>Why are <code>Thread.suspend</code> and
<code>Thread.resume</code> deprecated and the ability to suspend or
resume a thread removed?</h2>
<p><code>Thread.suspend</code> was inherently deadlock-prone. If the
target thread held a lock on a monitor protecting a critical
system resource when it is suspended, no thread could access the
resource until the target thread was resumed. If the thread intending
to resume the target thread attempted to lock the monitor prior
to calling <code>resume</code>, deadlock resulted. Such deadlocks
typically manifest themselves as "frozen" processes.</p>
<hr>
<h2>What should I use instead of <code>Thread.suspend</code> and
<code>Thread.resume</code>?</h2>
<p>As with <code>Thread.stop</code>, the prudent approach is to
have the "target thread" poll a variable indicating the desired
state of the thread (active or suspended). When the desired state
is suspended, the thread waits using <code>Object.wait</code>. When
the thread is resumed, the target thread is notified using
<code>Object.notify</code>.</p>
<p>For example, suppose your applet contains the following
mousePressed event handler, which toggles the state of a thread
called <code>blinker</code>:</p>
<pre>
private boolean threadSuspended;
Public void mousePressed(MouseEvent e) {
e.consume();
if (threadSuspended)
blinker.resume();
else
blinker.suspend(); // DEADLOCK-PRONE!
threadSuspended = !threadSuspended;
}
</pre>
You can avoid the use of <code>Thread.suspend</code> and
<code>Thread.resume</code> by replacing the event handler above
with:
<pre>
public synchronized void mousePressed(MouseEvent e) {
e.consume();
threadSuspended = !threadSuspended;
if (!threadSuspended)
notify();
}
</pre>
and adding the following code to the "run loop":
<pre>
synchronized(this) {
while (threadSuspended)
wait();
}
</pre>
The <code>wait</code> method throws the
<code>InterruptedException</code>, so it must be inside a <code>try
... catch</code> clause. It's fine to put it in the same clause as
the <code>sleep</code>. The check should follow (rather than
precede) the <code>sleep</code> so the window is immediately
repainted when the thread is "resumed." The resulting
<code>run</code> method follows:
<pre>
public void run() {
while (true) {
try {
Thread.sleep(interval);
synchronized(this) {
while (threadSuspended)
wait();
}
} catch (InterruptedException e){
}
repaint();
}
}
</pre>
Note that the <code>notify</code> in the <code>mousePressed</code>
method and the <code>wait</code> in the <code>run</code> method are
inside <code>synchronized</code> blocks. This is required by the
language, and ensures that <code>wait</code> and
<code>notify</code> are properly serialized. In practical terms,
this eliminates race conditions that could cause the "suspended"
thread to miss a <code>notify</code> and remain suspended
indefinitely.
<p>While the cost of synchronization in Java is decreasing as the
platform matures, it will never be free. A simple trick can be used
to remove the synchronization that we've added to each iteration of
the "run loop." The synchronized block that was added is replaced
by a slightly more complex piece of code that enters a synchronized
block only if the thread has actually been suspended:</p>
<pre>
if (threadSuspended) {
synchronized(this) {
while (threadSuspended)
wait();
}
}
</pre>
<p>In the absence of explicit synchronization,
<code>threadSuspended</code> must be made <code>volatile</code> to ensure
prompt communication of the suspend-request.</p>
The resulting <code>run</code> method is:
<pre>
private volatile boolean threadSuspended;
public void run() {
while (true) {
try {
Thread.sleep(interval);
if (threadSuspended) {
synchronized(this) {
while (threadSuspended)
wait();
}
}
} catch (InterruptedException e){
}
repaint();
}
}
</pre>
<hr>
<h2>Can I combine the two techniques to produce a thread that may
be safely "stopped" or "suspended"?</h2>
Yes, it's reasonably straightforward. The one subtlety is that the
target thread may already be suspended at the time that another
thread tries to stop it. If the <code>stop</code> method merely sets
the state variable (<code>blinker</code>) to null, the target thread
will remain suspended (waiting on the monitor), rather than exiting
gracefully as it should. If the applet is restarted, multiple
threads could end up waiting on the monitor at the same time,
resulting in erratic behavior.
<p>To rectify this situation, the <code>stop</code> method must ensure
that the target thread resumes immediately if it is suspended. Once
the target thread resumes, it must recognize immediately that it
has been stopped, and exit gracefully. Here's how the resulting
<code>run</code> and <code>stop</code> methods look:</p>
<pre>
public void run() {
Thread thisThread = Thread.currentThread();
while (blinker == thisThread) {
try {
Thread.sleep(interval);
synchronized(this) {
while (threadSuspended &amp;&amp; blinker==thisThread)
wait();
}
} catch (InterruptedException e){
}
repaint();
}
}
public synchronized void stop() {
blinker = null;
notify();
}
</pre>
If the <code>stop</code> method calls <code>Thread.interrupt</code>, as
described above, it needn't call <code>notify</code> as well, but it
still must be synchronized. This ensures that the target thread
won't miss an interrupt due to a race condition.
</body>
</html>

@ -533,12 +533,13 @@ public class ThreadInfo {
}
/**
* Tests if the thread associated with this {@code ThreadInfo}
* is suspended. This method returns {@code true} if
* {@link Thread#suspend} has been called.
* Tests if the thread associated with this {@code ThreadInfo} is
* <a href="{@docRoot}/../specs/jvmti.html#SuspendThread">suspended</a>.
*
* @return {@code true} if the thread is suspended;
* {@code false} otherwise.
*
* @spec jvmti.html JVM Tool Interface
*/
public boolean isSuspended() {
return suspended;

@ -22,60 +22,43 @@
*/
/* @test
* @bug 8289610 8249627 8205132
* @summary Test that Thread stop/suspend/resume throw UOE
* @run junit DegradedMethodsThrowUOE
* @bug 8289610 8249627 8205132 8320532
* @summary Test that Thread stops throws UOE
* @run junit ThreadStopTest
*/
import java.time.Duration;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.LockSupport;
import java.util.function.Consumer;
import java.util.stream.Stream;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.*;
class DegradedMethodsThrowUOE {
class ThreadStopTest {
/**
* Returns a stream of operations on a Thread that should throw UOE.
* Test current thread calling Thread.stop on itself.
*/
static Stream<Consumer<Thread>> ops() {
return Stream.<Consumer<Thread>>of(
Thread::stop,
Thread::suspend,
Thread::resume
);
}
/**
* Test degraded method on current thread.
*/
@ParameterizedTest
@MethodSource("ops")
void testCurrentThread(Consumer<Thread> op) {
@Test
void testCurrentThread() {
var thread = Thread.currentThread();
assertThrows(UnsupportedOperationException.class, () -> op.accept(thread));
assertThrows(UnsupportedOperationException.class, thread::stop);
}
/**
* Test degraded method on an unstarted thread.
* Test Thread.stop on an unstarted thread.
*/
@ParameterizedTest
@MethodSource("ops")
void testUnstartedThread(Consumer<Thread> op) {
@Test
void testUnstartedThread() {
Thread thread = new Thread(() -> { });
assertThrows(UnsupportedOperationException.class, () -> op.accept(thread));
assertThrows(UnsupportedOperationException.class, thread::stop);
assertTrue(thread.getState() == Thread.State.NEW);
}
/**
* Test degraded method on a thread spinning in a loop.
* Test Thread.stop on a thread spinning in a loop.
*/
@ParameterizedTest
@MethodSource("ops")
void testRunnableThread(Consumer<Thread> op) throws Exception {
@Test
void testRunnableThread() throws Exception {
AtomicBoolean done = new AtomicBoolean();
Thread thread = new Thread(() -> {
while (!done.get()) {
@ -84,7 +67,7 @@ class DegradedMethodsThrowUOE {
});
thread.start();
try {
assertThrows(UnsupportedOperationException.class, () -> op.accept(thread));
assertThrows(UnsupportedOperationException.class, thread::stop);
// thread should not terminate
boolean terminated = thread.join(Duration.ofMillis(500));
@ -96,11 +79,10 @@ class DegradedMethodsThrowUOE {
}
/**
* Test degraded method on a thread that is parked.
* Test Thread.stop on a thread that is parked.
*/
@ParameterizedTest
@MethodSource("ops")
void testWaitingThread(Consumer<Thread> op) throws Exception {
@Test
void testWaitingThread() throws Exception {
Thread thread = new Thread(LockSupport::park);
thread.start();
try {
@ -108,7 +90,7 @@ class DegradedMethodsThrowUOE {
while ((thread.getState() != Thread.State.WAITING)) {
Thread.sleep(10);
}
assertThrows(UnsupportedOperationException.class, () -> op.accept(thread));
assertThrows(UnsupportedOperationException.class, thread::stop);
assertTrue(thread.getState() == Thread.State.WAITING);
} finally {
LockSupport.unpark(thread);
@ -117,15 +99,14 @@ class DegradedMethodsThrowUOE {
}
/**
* Test degraded method on a terminated thread.
* Test Thread.stop on a terminated thread.
*/
@ParameterizedTest
@MethodSource("ops")
void testTerminatedThread(Consumer<Thread> op) throws Exception {
@Test
void testTerminatedThread() throws Exception {
Thread thread = new Thread(() -> { });
thread.start();
thread.join();
assertThrows(UnsupportedOperationException.class, () -> op.accept(thread));
assertThrows(UnsupportedOperationException.class, thread::stop);
assertTrue(thread.getState() == Thread.State.TERMINATED);
}
}

@ -273,64 +273,6 @@ class ThreadAPI {
}
}
/**
* Test Thread::suspend from current thread.
*/
@Test
void testSuspend1() throws Exception {
VThreadRunner.run(() -> {
Thread t = Thread.currentThread();
assertThrows(UnsupportedOperationException.class, t::suspend);
});
}
/**
* Test Thread::suspend from another thread.
*/
@Test
void testSuspend2() throws Exception {
var thread = Thread.ofVirtual().start(() -> {
try {
Thread.sleep(20*1000);
} catch (InterruptedException e) { }
});
try {
assertThrows(UnsupportedOperationException.class, () -> thread.suspend());
} finally {
thread.interrupt();
thread.join();
}
}
/**
* Test Thread::resume from current thread.
*/
@Test
void testResume1() throws Exception {
VThreadRunner.run(() -> {
Thread t = Thread.currentThread();
assertThrows(UnsupportedOperationException.class, t::resume);
});
}
/**
* Test Thread::resume from another thread.
*/
@Test
void testResume2() throws Exception {
var thread = Thread.ofVirtual().start(() -> {
try {
Thread.sleep(20*1000);
} catch (InterruptedException e) { }
});
try {
assertThrows(UnsupportedOperationException.class, () -> thread.resume());
} finally {
thread.interrupt();
thread.join();
}
}
/**
* Test Thread.join before thread starts, platform thread invokes join.
*/

@ -732,18 +732,6 @@ class BasicTests {
group.list();
}
@Test
void testSuspend() {
ThreadGroup group = new ThreadGroup("foo");
assertThrows(UnsupportedOperationException.class, () -> group.suspend());
}
@Test
void testResume() {
ThreadGroup group = new ThreadGroup("foo");
assertThrows(UnsupportedOperationException.class, () -> group.resume());
}
@Test
void testStop() {
ThreadGroup group = new ThreadGroup("foo");

@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -64,7 +64,9 @@ public class SendUrgentData {
boolean inline = false;
if (args.length > 0 && args[0].equals("-server")) {
System.out.println(serverThread.getAddress());
Thread.currentThread().suspend();
while (true) {
Thread.sleep(60_000);
}
} else {
if (args.length > 0 && args[0].equals("-client")) {
host = args[1];