Skip to content

Commit

Permalink
8289610: Degrade Thread.stop
Browse files Browse the repository at this point in the history
Reviewed-by: rriggs, cjplummer, jpai, mchung, prr, mullan
  • Loading branch information
Alan Bateman committed Sep 23, 2022
1 parent 05c8cab commit acd5bcf
Show file tree
Hide file tree
Showing 26 changed files with 179 additions and 226 deletions.
8 changes: 0 additions & 8 deletions src/java.base/share/classes/java/io/FilterOutputStream.java
Expand Up @@ -190,17 +190,9 @@ public void close() throws IOException {
try {
out.close();
} catch (Throwable closeException) {
// evaluate possible precedence of flushException over closeException
if ((flushException instanceof ThreadDeath) &&
!(closeException instanceof ThreadDeath)) {
flushException.addSuppressed(closeException);
throw (ThreadDeath) flushException;
}

if (flushException != closeException) {
closeException.addSuppressed(flushException);
}

throw closeException;
}
}
Expand Down
19 changes: 4 additions & 15 deletions src/java.base/share/classes/java/io/ObjectInputStream.java
Expand Up @@ -2430,8 +2430,6 @@ private void readSerialData(Object obj, ObjectStreamClass desc)
// Read fields of the current descriptor into a new FieldValues and discard
new FieldValues(slotDesc, true);
} else if (slotDesc.hasReadObjectMethod()) {
ThreadDeath t = null;
boolean reset = false;
SerialCallbackContext oldContext = curContext;
if (oldContext != null)
oldContext.check();
Expand All @@ -2450,19 +2448,10 @@ private void readSerialData(Object obj, ObjectStreamClass desc)
*/
handles.markException(passHandle, ex);
} finally {
do {
try {
curContext.setUsed();
if (oldContext!= null)
oldContext.check();
curContext = oldContext;
reset = true;
} catch (ThreadDeath x) {
t = x; // defer until reset is true
}
} while (!reset);
if (t != null)
throw t;
curContext.setUsed();
if (oldContext!= null)
oldContext.check();
curContext = oldContext;
}

/*
Expand Down
6 changes: 1 addition & 5 deletions src/java.base/share/classes/java/lang/Error.java
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1995, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1995, 2022, 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
Expand Down Expand Up @@ -29,9 +29,6 @@
* An {@code Error} is a subclass of {@code Throwable}
* that indicates serious problems that a reasonable application
* should not try to catch. Most such errors are abnormal conditions.
* The {@code ThreadDeath} error, though a "normal" condition,
* is also a subclass of {@code Error} because most applications
* should not try to catch it.
* <p>
* A method is not required to declare in its {@code throws}
* clause any subclasses of {@code Error} that might be thrown
Expand All @@ -42,7 +39,6 @@
* exceptions for the purposes of compile-time checking of exceptions.
*
* @author Frank Yellin
* @see java.lang.ThreadDeath
* @jls 11.2 Compile-Time Checking of Exceptions
* @since 1.0
*/
Expand Down
12 changes: 1 addition & 11 deletions src/java.base/share/classes/java/lang/RuntimePermission.java
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2022, 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
Expand Down Expand Up @@ -179,16 +179,6 @@
* </tr>
*
* <tr>
* <th scope="row">stopThread</th>
* <td>Stopping of threads via calls to the Thread {@code stop}
* method</td>
* <td>This allows code to stop any thread in the system provided that it is
* already granted permission to access that thread.
* This poses as a threat, because that code may corrupt the system by
* killing existing threads.</td>
* </tr>
*
* <tr>
* <th scope="row">modifyThreadGroup</th>
* <td>modification of thread groups, e.g., via calls to ThreadGroup
* {@code destroy}, {@code getParent}, {@code resume},
Expand Down
4 changes: 1 addition & 3 deletions src/java.base/share/classes/java/lang/Shutdown.java
Expand Up @@ -129,9 +129,7 @@ private static void runHooks() {
}
if (hook != null) hook.run();
} catch (Throwable t) {
if (t instanceof ThreadDeath td) {
throw td;
}
// ignore
}
}

Expand Down
84 changes: 12 additions & 72 deletions src/java.base/share/classes/java/lang/Thread.java
Expand Up @@ -1629,59 +1629,19 @@ private void exit() {
}

/**
* Forces the thread to stop executing.
* <p>
* If there is a security manager installed, its {@code checkAccess}
* method is called with {@code this}
* as its argument. This may result in a
* {@code SecurityException} being raised (in the current thread).
* <p>
* If this thread is different from the current thread (that is, the current
* thread is trying to stop a thread other than itself), the
* security manager's {@code checkPermission} method (with a
* {@code RuntimePermission("stopThread")} argument) is called in
* addition.
* Again, this may result in throwing a
* {@code SecurityException} (in the current thread).
* <p>
* The thread represented by this thread is forced to stop whatever
* it is doing abnormally and to throw a newly created
* {@code ThreadDeath} object as an exception.
* <p>
* It is permitted to stop a thread that has not yet been started.
* If the thread is eventually started, it immediately terminates.
* <p>
* An application should not normally try to catch
* {@code ThreadDeath} unless it must do some extraordinary
* cleanup operation (note that the throwing of
* {@code ThreadDeath} causes {@code finally} clauses of
* {@code try} statements to be executed before the thread
* officially terminates). If a {@code catch} clause catches a
* {@code ThreadDeath} object, it is important to rethrow the
* object so that the thread actually terminates.
* <p>
* The top-level error handler that reacts to otherwise uncaught
* exceptions does not print out a message or otherwise notify the
* application if the uncaught exception is an instance of
* {@code ThreadDeath}.
* Throws {@code UnsupportedOperationException}.
*
* @throws SecurityException if the current thread cannot
* modify this thread.
* @throws UnsupportedOperationException if invoked on a virtual thread
* @see #interrupt()
* @see #checkAccess()
* @see ThreadDeath
* @see ThreadGroup#uncaughtException(Thread,Throwable)
* @see SecurityManager#checkAccess(Thread)
* @see SecurityManager#checkPermission
* @deprecated This method is inherently unsafe. Stopping a thread with
* Thread.stop causes it to unlock all of the monitors that it
* has locked (as a natural consequence of the unchecked
* {@code ThreadDeath} exception propagating up the stack). If
* @throws UnsupportedOperationException always
*
* @deprecated This method was originally specified to "stop" a victim
* thread by causing the victim thread to throw a {@link ThreadDeath}.
* It was inherently unsafe. Stopping a thread caused it to unlock
* all of the monitors that it had locked (as a natural consequence
* of the {@code ThreadDeath} exception propagating up the stack). If
* any of the objects previously protected by these monitors were in
* an inconsistent state, the damaged objects become visible to
* other threads, potentially resulting in arbitrary behavior. Many
* uses of {@code stop} should be replaced by code that simply
* an inconsistent state, the damaged objects became visible to
* other threads, potentially resulting in arbitrary behavior.
* Usages of {@code stop} should be replaced by code that simply
* modifies some variable to indicate that the target thread should
* stop running. The target thread should check this variable
* regularly, and return from its run method in an orderly fashion
Expand All @@ -1695,26 +1655,7 @@ private void exit() {
*/
@Deprecated(since="1.2", forRemoval=true)
public final void stop() {
@SuppressWarnings("removal")
SecurityManager security = System.getSecurityManager();
if (security != null) {
checkAccess();
if (this != Thread.currentThread()) {
security.checkPermission(SecurityConstants.STOP_THREAD_PERMISSION);
}
}

if (isVirtual())
throw new UnsupportedOperationException();

// A zero status value corresponds to "NEW", it can't change to
// not-NEW because we hold the lock.
if (holder.threadStatus != 0) {
resume(); // Wake up thread if it was suspended; no-op otherwise
}

// The VM can handle all thread states
stop0(new ThreadDeath());
throw new UnsupportedOperationException();
}

/**
Expand Down Expand Up @@ -3094,7 +3035,6 @@ static void setHeadStackableScope(StackableScope scope) {

/* Some private helper methods */
private native void setPriority0(int newPriority);
private native void stop0(Object o);
private native void suspend0();
private native void resume0();
private native void interrupt0();
Expand Down
27 changes: 10 additions & 17 deletions src/java.base/share/classes/java/lang/ThreadDeath.java
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1995, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1995, 2022, 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
Expand All @@ -26,26 +26,19 @@
package java.lang;

/**
* An instance of {@code ThreadDeath} is thrown in the victim thread
* when the (deprecated) {@link Thread#stop()} method is invoked.
* An instance of {@code ThreadDeath} was originally specified to be thrown
* by a victim thread when "stopped" with {@link Thread#stop()}.
*
* <p>An application should catch instances of this class only if it
* must clean up after being terminated asynchronously. If
* {@code ThreadDeath} is caught by a method, it is important that it
* be rethrown so that the thread actually dies.
*
* <p>The {@linkplain ThreadGroup#uncaughtException top-level error
* handler} does not print out a message if {@code ThreadDeath} is
* never caught.
*
* <p>The class {@code ThreadDeath} is specifically a subclass of
* {@code Error} rather than {@code Exception}, even though it is a
* "normal occurrence", because many applications catch all
* occurrences of {@code Exception} and then discard the exception.
* @deprecated {@link Thread#stop()} was originally specified to "stop" a victim
* thread by causing the victim thread to throw a {@code ThreadDeath}. It
* was inherently unsafe and deprecated in an early JDK release. The ability
* to "stop" a thread with {@code Thread.stop} has been removed and the
* {@code Thread.stop} method changed to throw an exception. Consequently,
* {@code ThreadDeath} is also deprecated, for removal.
*
* @since 1.0
*/

@Deprecated(since="20", forRemoval=true)
public class ThreadDeath extends Error {
@java.io.Serial
private static final long serialVersionUID = -4417128565033088268L;
Expand Down
14 changes: 5 additions & 9 deletions src/java.base/share/classes/java/lang/ThreadGroup.java
Expand Up @@ -674,12 +674,9 @@ private void list(Map<ThreadGroup, List<Thread>> map, PrintStream out, int inden
* uncaught exception handler} installed, and if so, its
* {@code uncaughtException} method is called with the same
* two arguments.
* <li>Otherwise, this method determines if the {@code Throwable}
* argument is an instance of {@link ThreadDeath}. If so, nothing
* special is done. Otherwise, a message containing the
* thread's name, as returned from the thread's {@link
* Thread#getName getName} method, and a stack backtrace,
* using the {@code Throwable}'s {@link
* <li>Otherwise, a message containing the thread's name, as returned
* from the thread's {@link Thread#getName getName} method, and a
* stack backtrace, using the {@code Throwable}'s {@link
* Throwable#printStackTrace() printStackTrace} method, is
* printed to the {@linkplain System#err standard error stream}.
* </ul>
Expand All @@ -699,9 +696,8 @@ public void uncaughtException(Thread t, Throwable e) {
Thread.getDefaultUncaughtExceptionHandler();
if (ueh != null) {
ueh.uncaughtException(t, e);
} else if (!(e instanceof ThreadDeath)) {
System.err.print("Exception in thread \""
+ t.getName() + "\" ");
} else {
System.err.print("Exception in thread \"" + t.getName() + "\" ");
e.printStackTrace(System.err);
}
}
Expand Down
@@ -1,6 +1,6 @@
<!doctype html>
<!--
Copyright (c) 2005, 2019, Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2005, 2022, 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
Expand Down Expand Up @@ -31,31 +31,32 @@
<body>
<h1>Java Thread Primitive Deprecation</h1>
<hr>
<h2>Why is <code>Thread.stop</code> deprecated?</h2>
<p>Because it is inherently unsafe. Stopping a thread causes it to
unlock all the monitors that it has locked. (The monitors are
unlocked as the <code>ThreadDeath</code> exception propagates up
<h2>Why is <code>Thread.stop</code> deprecated and the ability to
stop a thread removed?</h2>
<p>Because it was inherently unsafe. Stopping a thread caused it to
unlock all the monitors that it had locked. (The monitors were
unlocked as the <code>ThreadDeath</code> exception propagated up
the stack.) If any of the objects previously protected by these
monitors were in an inconsistent state, other threads may now view
monitors were in an inconsistent state, other threads may have viewed
these objects in an inconsistent state. Such objects are said to be
<i>damaged</i>. When threads operate on damaged objects, arbitrary
behavior can result. This behavior may be subtle and difficult to
detect, or it may be pronounced. Unlike other unchecked exceptions,
<code>ThreadDeath</code> kills threads silently; thus, the user has
no warning that his program may be corrupted. The corruption can
<code>ThreadDeath</code> killed threads silently; thus, the user had
no warning that their program may be corrupted. The corruption could
manifest itself at any time after the actual damage occurs, even
hours or days in the future.</p>
<hr>
<h2>Couldn't I just catch the <code>ThreadDeath</code> exception
and fix the damaged object?</h2>
<h2>Couldn't I have just caught <code>ThreadDeath</code> and fixed
the damaged object?</h2>
<p>In theory, perhaps, but it would <em>vastly</em> complicate the
task of writing correct multithreaded code. The task would be
nearly insurmountable for two reasons:</p>
<ol>
<li>A thread can throw a <code>ThreadDeath</code> exception
<li>A thread could throw a <code>ThreadDeath</code> exception
<i>almost anywhere</i>. All synchronized methods and blocks would
have to be studied in great detail, with this in mind.</li>
<li>A thread can throw a second <code>ThreadDeath</code> exception
<li>A thread could throw a second <code>ThreadDeath</code> exception
while cleaning up from the first (in the <code>catch</code> or
<code>finally</code> clause). Cleanup would have to be repeated till
it succeeded. The code to ensure this would be quite complex.</li>
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2022, 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
Expand Down Expand Up @@ -178,7 +178,7 @@ else if (info.getClass() == int[].class) {
catch (Error e) {
// Pass through an Error, including BootstrapMethodError, any other
// form of linkage error, such as IllegalAccessError if the bootstrap
// method is inaccessible, or say ThreadDeath/OutOfMemoryError
// method is inaccessible, or say OutOfMemoryError
// See the "Linking Exceptions" section for the invokedynamic
// instruction in JVMS 6.5.
throw e;
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/invoke/CallSite.java
Expand Up @@ -335,7 +335,7 @@ static CallSite makeSite(MethodHandle bootstrapMethod,
} catch (Error e) {
// Pass through an Error, including BootstrapMethodError, any other
// form of linkage error, such as IllegalAccessError if the bootstrap
// method is inaccessible, or say ThreadDeath/OutOfMemoryError
// method is inaccessible, or say OutOfMemoryError
// See the "Linking Exceptions" section for the invokedynamic
// instruction in JVMS 6.5.
throw e;
Expand Down
Expand Up @@ -124,10 +124,6 @@ private SecurityConstants () {
public static final RuntimePermission GET_CLASSLOADER_PERMISSION =
new RuntimePermission("getClassLoader");

// java.lang.Thread
public static final RuntimePermission STOP_THREAD_PERMISSION =
new RuntimePermission("stopThread");

// java.lang.Thread
public static final RuntimePermission GET_STACK_TRACE_PERMISSION =
new RuntimePermission("getStackTrace");
Expand Down
1 change: 0 additions & 1 deletion src/java.base/share/native/libjava/Thread.c
Expand Up @@ -37,7 +37,6 @@

static JNINativeMethod methods[] = {
{"start0", "()V", (void *)&JVM_StartThread},
{"stop0", "(" OBJ ")V", (void *)&JVM_StopThread},
{"isAlive0", "()Z", (void *)&JVM_IsThreadAlive},
{"suspend0", "()V", (void *)&JVM_SuspendThread},
{"resume0", "()V", (void *)&JVM_ResumeThread},
Expand Down

1 comment on commit acd5bcf

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.