8277861: Terminally deprecate Thread.stop

[AlanBateman]

Thread.stop is inherently unsafe and has been deprecated since Java 1.2 (1998). It's time to terminally deprecate this method so it can be degraded and removed in the future.

This PR does not propose any changes to the JVM TI StopThread function (or the corresponding JDWP command or JDI method).

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issues

• JDK-8277861: Terminally deprecate Thread.stop
• JDK-8277987: Terminally deprecate Thread.stop (CSR) ⚠️ Issue is not open.

Reviewers

• Roger Riggs (@RogerRiggs - Reviewer)
• Mandy Chung (@mlchung - Reviewer)
• Uwe Schindler (@uschindler - Author)
• David Holmes (@dholmes-ora - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/6616/head:pull/6616
$ git checkout pull/6616

Update a local copy of the PR:
$ git checkout pull/6616
$ git pull https://git.openjdk.java.net/jdk pull/6616/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 6616

View PR using the GUI difftool:
$ git pr show -t 6616

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/6616.diff

[AlanBateman]

/csr

[bridgekeeper]

👋 Welcome back alanb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@AlanBateman has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@AlanBateman please create a CSR request for issue JDK-8277861. This pull request cannot be integrated until the CSR request is approved.

[openjdk]

@AlanBateman The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (b15ec80f)

[RogerRiggs]

Past time for this to go.

[mlbridge]

Mailing list message from Alan Snyder on core-libs-dev:

Although I understand the potential dangers of using Thread.stop, it seems to me there are cases where its use is legitimate and valuable.

The examples I am thinking of involve a potentially long running computation whose result is no longer needed.
In particular, I am thinking of pure computations such as image analysis or audio analysis that do not involve waiting (so that interrupt is not useful)
and probably are implemented using some C library (which is not feasible to modify to insert code to support graceful interruption).

Is there some alternative that can be used in such cases?

Perhaps a version of stop() that only works if no locks are held?

Alan

On Nov 30, 2021, at 7:51 AM, Roger Riggs <rriggs at openjdk.java.net> wrote:

On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:

Thread.stop is inherently unsafe and has been deprecated since Java 1.2 (1998). It's time to terminally deprecate this method so it can be degraded and removed in the future.

This PR does not propose any changes to the JVM TI StopThread function (or the corresponding JDWP command or JDI method).

Past time for this to go.

[mlbridge]

Mailing list message from Alan Bateman on core-libs-dev:

On 30/11/2021 17:13, Alan Snyder wrote:

Although I understand the potential dangers of using Thread.stop, it seems to me there are cases where its use is legitimate and valuable.

The examples I am thinking of involve a potentially long running computation whose result is no longer needed.
In particular, I am thinking of pure computations such as image analysis or audio analysis that do not involve waiting (so that interrupt is not useful)
and probably are implemented using some C library (which is not feasible to modify to insert code to support graceful interruption).

JCiP Ch.7 has some good advice on this topic. In general, it needs the
task to poll a cancel status or test
Thread.currentThread().isInterrupted() to check for interrupt. In your
scenario, with image analysis in native code, then Thread.stop won't
help as it would need to return from the native code to detect the async
exception.

-Alan

[uschindler]

Let's this go away! 😍

[mlbridge]

Mailing list message from Alan Snyder on core-libs-dev:

I think you are saying that to kill a thread running native code I would need to use native code. Is that right?

On Nov 30, 2021, at 10:17 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:

On 30/11/2021 17:13, Alan Snyder wrote:

Although I understand the potential dangers of using Thread.stop, it seems to me there are cases where its use is legitimate and valuable.

The examples I am thinking of involve a potentially long running computation whose result is no longer needed.
In particular, I am thinking of pure computations such as image analysis or audio analysis that do not involve waiting (so that interrupt is not useful)
and probably are implemented using some C library (which is not feasible to modify to insert code to support graceful interruption).

JCiP Ch.7 has some good advice on this topic. In general, it needs the task to poll a cancel status or test Thread.currentThread().isInterrupted() to check for interrupt. In your scenario, with image analysis in native code, then Thread.stop won't help as it would need to return from the native code to detect the async exception.

-Alan

[openjdk]

@AlanBateman This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8277861: Terminally deprecate Thread.stop

Reviewed-by: rriggs, mchung, uschindler, dholmes

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 13 new commits pushed to the master branch:

• da2be99: 8277026: Remove blank lines remaining from snippet markup
• 0a01baa: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime
• 7049c13: 8231107: Allow store password to be null when saving a PKCS12 KeyStore
• ab867f6: 8272162: S4U2Self ticket without forwardable flag
• dd73e3c: 8277814: ConcurrentRefineThread should report rate when deactivating
• 65251f7: 8151594: Move concurrent refinement thread activation logging out of GC pause
• f1c20e9: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently
• 2942646: 8276683: Malformed Javadoc inline tags in JDK source in com/sun/tools/javac/util/RawDiagnosticFormatter.java
• e30e676: 8277606: String(String) constructor could copy hashIsZero
• 5a4a9bb: 8278019: ProblemList java/awt/dnd/BadSerializationTest/BadSerializationTest.java on linux and windows
• ... and 3 more: https://git.openjdk.java.net/jdk/compare/915084041f32bf6ffe4d12c031ac5e69adcc07f5...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[dholmes-ora]

Approved with extreme prejudice. :)

[AlanBateman]

/integrate

[openjdk]

Going to push as commit fde0b95.
Since your change was applied there have been 16 commits pushed to the master branch:

• 70d5dff: 8275326: C2: assert(no_dead_loop) failed: dead loop detected
• 349328c: 8277777: [Vector API] assert(r->is_XMMRegister()) failed: must be in x86_32.ad
• c733193: 8262297: ImageIO.write() method will throw IndexOutOfBoundsException
• da2be99: 8277026: Remove blank lines remaining from snippet markup
• 0a01baa: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime
• 7049c13: 8231107: Allow store password to be null when saving a PKCS12 KeyStore
• ab867f6: 8272162: S4U2Self ticket without forwardable flag
• dd73e3c: 8277814: ConcurrentRefineThread should report rate when deactivating
• 65251f7: 8151594: Move concurrent refinement thread activation logging out of GC pause
• f1c20e9: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently
• ... and 6 more: https://git.openjdk.java.net/jdk/compare/915084041f32bf6ffe4d12c031ac5e69adcc07f5...master

Your commit was automatically rebased without conflicts.

[openjdk]

@AlanBateman Pushed as commit fde0b95.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[mlbridge]

Mailing list message from David Holmes on core-libs-dev:

On 1/12/2021 3:13 am, Alan Snyder wrote:

Although I understand the potential dangers of using Thread.stop, it seems to me there are cases where its use is legitimate and valuable.

No there really aren't. :) The perceived utility of stop() is an
illusion. It is almost impossible to write any non-trivial code that is
async-exception-safe and no JDK library code is written to be
async-exception-safe including thread tear-down. So while you can say
"stop() is the only way to disrupt this piece of code", you cannot
ensure that it is disrupted safely. Once stop is used you need to throw
away _all_ stateful objects that may have been in active use while
ThreadDeath was propagated. And even during propagation you can easily
trigger secondary exceptions.

Cheers,
David

The examples I am thinking of involve a potentially long running computation whose result is no longer needed.
In particular, I am thinking of pure computations such as image analysis or audio analysis that do not involve waiting (so that interrupt is not useful)
and probably are implemented using some C library (which is not feasible to modify to insert code to support graceful interruption).

Is there some alternative that can be used in such cases?

Perhaps a version of stop() that only works if no locks are held?

Alan

On Nov 30, 2021, at 7:51 AM, Roger Riggs <rriggs at openjdk.java.net> wrote:

On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:

Thread.stop is inherently unsafe and has been deprecated since Java 1.2 (1998). It's time to terminally deprecate this method so it can be degraded and removed in the future.

This PR does not propose any changes to the JVM TI StopThread function (or the corresponding JDWP command or JDI method).

Past time for this to go.

[mlbridge]

Mailing list message from Alan Snyder on core-libs-dev:

It is almost impossible to write any non-trivial code that is async-exception-safe and no JDK library code is written to be async-exception-safe including thread tear-down. So while you can say "stop() is the only way to disrupt this piece of code", you cannot ensure that it is disrupted safely. Once stop is used you need to throw away _all_ stateful objects that may have been in active use while ThreadDeath was propagated. And even during propagation you can easily trigger secondary exceptions.

It seems that it should be possible to stop a thread spawned to execute code in a native library that works on data in native memory..
If what you say about thread tear-down is true, then I guess I would need to spawn the thread from native code as well.

On Nov 30, 2021, at 5:58 PM, David Holmes <david.holmes at oracle.com> wrote:

On 1/12/2021 3:13 am, Alan Snyder wrote:

Although I understand the potential dangers of using Thread.stop, it seems to me there are cases where its use is legitimate and valuable.

No there really aren't. :) The perceived utility of stop() is an illusion. It is almost impossible to write any non-trivial code that is async-exception-safe and no JDK library code is written to be async-exception-safe including thread tear-down. So while you can say "stop() is the only way to disrupt this piece of code", you cannot ensure that it is disrupted safely. Once stop is used you need to throw away _all_ stateful objects that may have been in active use while ThreadDeath was propagated. And even during propagation you can easily trigger secondary exceptions.

Cheers,
David

The examples I am thinking of involve a potentially long running computation whose result is no longer needed.
In particular, I am thinking of pure computations such as image analysis or audio analysis that do not involve waiting (so that interrupt is not useful)
and probably are implemented using some C library (which is not feasible to modify to insert code to support graceful interruption).
Is there some alternative that can be used in such cases?
Perhaps a version of stop() that only works if no locks are held?
Alan

On Nov 30, 2021, at 7:51 AM, Roger Riggs <rriggs at openjdk.java.net> wrote:

On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:

Thread.stop is inherently unsafe and has been deprecated since Java 1.2 (1998). It's time to terminally deprecate this method so it can be degraded and removed in the future.

This PR does not propose any changes to the JVM TI StopThread function (or the corresponding JDWP command or JDI method).

Past time for this to go.

[stuart-marks]

Endorsed.