RFR: 8289610: Degrade Thread.stop [v4]

[David Holmes]

On Sat, 17 Sep 2022 08:41:19 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Degrade Thread.stop to throw UOE unconditionally, deprecate ThreadDeath for removal, and remove the remaining special handling of ThreadDeath from the JDK.
>> 
>> Thread.stop is inherently unsafe and has been deprecated since JDK 1.2 (1998) with a link to a supplementary page that explains the rationale. Some of the API surface has already been degraded or removed: Thread.stop(Throwable) was degraded to throw UOE in Java 8 and removed in Java 11, and ThreadGroup.stop was degraded to throw UOE in Java 19. As of Java 19, the no-arg Thread.stop continues to work as before for platform threads but throws UOE for virtual threads. The next step in the glacial pace removal is the degrading of the no-arg Thread.stop method to throw UOE for all threads.
>> 
>> To keep things manageable, the change proposed here leaves JVM_StopThread in place. A separate issue will remove it and do other cleanup/removal in the VM. We have another JBS issue for the updates to the JLS and JVMS where asynchronous exceptions are defined. There is also some remaining work on a test class used by 6 jshell tests - if they aren't done in time then we will temporarily exclude them.
>> 
>> The change here has no  impact on the debugger APIs (JVM TI StopThread, JDWP ThreadReference/Stop, and JDI ThreadReference.stop). Debuggers can continue to cause threads to throw an asynchronous exception, as might be done when simulating code throwing an exception at some point in the code.
>
> Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision:
> 
>  - Merge
>  - Updates to Java Thread Primitive Deprecation page
>  - Repalce "it" with "victim thread"
>  - Merge
>  - Revert test/langtools/ProblemList.txt as jshell tests no longer rely on Thread.stop
>  - become -> became in javadoc
>  - Merge
>  - Remove stopThread permission from RuntimePermission table, exclude jshell tests
>  - Deprecate for removal
>  - Next iteration, update dates in headers
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/4a31f682...93806f99

Hi Alan,

Good to see this finally become non-functional but as long as the debugger/JVMTI can trigger throwing an async ThreadDeath then I have to question the potential change in behaviour introduced by removing all the `catch (ThreadDeath td)` logic in various places.

src/java.base/share/classes/java/io/FilterOutputStream.java line 195:

> 193:                    // evaluate possible precedence of flushException over closeException
> 194:                    if ((flushException instanceof ThreadDeath) &&
> 195:                        !(closeException instanceof ThreadDeath)) {

If the ThreadDeath originates from the debugger then this is now a change in behaviour.

src/java.base/share/classes/java/lang/Shutdown.java line 132:

> 130:                 if (hook != null) hook.run();
> 131:             } catch (Throwable t) {
> 132:                 // ignore

Again change of behaviour if TD originates from debugger.

src/java.base/share/classes/java/lang/ThreadDeath.java line 30:

> 28: /**
> 29:  * An instance of {@code ThreadDeath} was originally specified to be thrown
> 30:  * by a victim thread when "stopped" with {@link Thread#stop()}.

Should this have always mentioned the possibility of TD coming from a debugger as well?

src/java.base/share/classes/java/lang/ThreadGroup.java line 700:

> 698:                 ueh.uncaughtException(t, e);
> 699:             } else {
> 700:                 System.err.print("Exception in thread \"" + t.getName() + "\" ");

Again change in behaviour.

src/java.base/share/classes/java/lang/doc-files/threadPrimitiveDeprecation.html line 37:

> 35: stop a thread removed?</h2>
> 36: <p>Because it was inherently unsafe. Stopping a thread caused it to
> 37: unlock all the monitors that it had locked. (The monitors were

Just an aside but this rationale was always a significant under-statement as async-exceptions are inherently unsafe even if no concurrency or monitors are involved.

src/java.desktop/macosx/classes/sun/lwawt/LWToolkit.java line 170:

> 168:                     }
> 169:                 }
> 170:             } catch (ThreadDeath td) {

Again change in behaviour

src/java.desktop/share/classes/java/awt/EventDispatchThread.java line 205:

> 203:             eq.dispatchEvent(event);
> 204:         }
> 205:         catch (ThreadDeath death) {

Again change in behaviour.

-------------

PR: https://git.openjdk.org/jdk/pull/10230