RFR(s): 8204243: remove Thread.destroy() and Thread.stop(Throwable)
Hi all, Please review this changeset to remove the Thread.destroy() and Thread.stop(Throwable) methods. Both of these methods have been deprecated for a long time, and they were deprecated for removal in Java SE 9. Thread.destroy() was never implemented, and has always thrown NoSuchMethodError back to the caller. Thread.stop(Throwable) was made non-functional in JDK 8, throwing UnsupportedOperationException back to the caller. Note that the no-arg Thread.stop() method remains deprecated, not for removal, and is unaffected by this changeset. Note also that ThreadGroup.destroy() doesn't actually destroy any threads -- just thread groups -- and that it is also unaffected by this changeset. Bug: https://bugs.openjdk.java.net/browse/JDK-8204243 Webrev: http://cr.openjdk.java.net/~smarks/reviews/8204243/webrev.0/ Thanks, s'marks aka @DrDeprecator
Hi Stuart, The changes look fine. Is there a CSR or a release note that also needs to be reviewed? Best Lance
On Jun 1, 2018, at 5:16 PM, Stuart Marks <stuart.marks@oracle.com> wrote:
Hi all,
Please review this changeset to remove the Thread.destroy() and Thread.stop(Throwable) methods. Both of these methods have been deprecated for a long time, and they were deprecated for removal in Java SE 9.
Thread.destroy() was never implemented, and has always thrown NoSuchMethodError back to the caller. Thread.stop(Throwable) was made non-functional in JDK 8, throwing UnsupportedOperationException back to the caller.
Note that the no-arg Thread.stop() method remains deprecated, not for removal, and is unaffected by this changeset.
Note also that ThreadGroup.destroy() doesn't actually destroy any threads -- just thread groups -- and that it is also unaffected by this changeset.
Bug:
https://bugs.openjdk.java.net/browse/JDK-8204243
Webrev:
http://cr.openjdk.java.net/~smarks/reviews/8204243/webrev.0/
Thanks,
s'marks aka @DrDeprecator
<http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen@oracle.com <mailto:Lance.Andersen@oracle.com>
On 6/1/18 2:37 PM, Lance Andersen wrote:
The changes look fine. Is there a CSR or a release note that also needs to be reviewed?
Thanks. No CSR yet. I usually do those after the code review. Good point about a release note, though; I'll add the label and subtask for one. s'marks
On 2/06/2018 9:53 AM, Stuart Marks wrote:
On 6/1/18 2:37 PM, Lance Andersen wrote:
The changes look fine. Is there a CSR or a release note that also needs to be reviewed?
Thanks. No CSR yet. I usually do those after the code review. Good point about a release note, though; I'll add the label and subtask for one.
I would expect the CSR that marked them as deprecated for removal, also serves for the actual removal. Certainly for VM flags we don't do a separate CSR for each phase (deprecation, obsoletion, expiration). David
s'marks
On 6/1/18 5:15 PM, David Holmes wrote:
I would expect the CSR that marked them as deprecated for removal, also serves for the actual removal. Certainly for VM flags we don't do a separate CSR for each phase (deprecation, obsoletion, expiration).
Hm. Well, this hasn't been tested for Java SE APIs yet, as most of the deprecations-for-removal occurred in Java SE 9, before the CSR was active. Instead, those deprecations went through the (Oracle internal) CCC process. Now that we're fully on the CSR system, I'd expect that deprecations (whether or not for removal) and removals of Java SE APIs would have separate CSR requests. The reason is that adding or changing a deprecation annotation is a spec change, and removing the API is a distinct spec change. They also occur in different releases. I can easily see that a different procedure would be followed for VM flags, though, since they aren't part of Java SE. s'marks
On 2/06/2018 11:07 AM, Stuart Marks wrote:
On 6/1/18 5:15 PM, David Holmes wrote:
I would expect the CSR that marked them as deprecated for removal, also serves for the actual removal. Certainly for VM flags we don't do a separate CSR for each phase (deprecation, obsoletion, expiration).
Hm. Well, this hasn't been tested for Java SE APIs yet, as most of the deprecations-for-removal occurred in Java SE 9, before the CSR was active. Instead, those deprecations went through the (Oracle internal) CCC process.
Now that we're fully on the CSR system, I'd expect that deprecations (whether or not for removal) and removals of Java SE APIs would have separate CSR requests. The reason is that adding or changing a deprecation annotation is a spec change, and removing the API is a distinct spec change. They also occur in different releases.
Good points. Though given the annotation is on the method being removed it's really only one spec change. Cheers, David
I can easily see that a different procedure would be followed for VM flags, though, since they aren't part of Java SE.
s'marks
On 02/06/2018 06:45, David Holmes wrote:
On 2/06/2018 11:07 AM, Stuart Marks wrote:
On 6/1/18 5:15 PM, David Holmes wrote:
I would expect the CSR that marked them as deprecated for removal, also serves for the actual removal. Certainly for VM flags we don't do a separate CSR for each phase (deprecation, obsoletion, expiration).
Hm. Well, this hasn't been tested for Java SE APIs yet, as most of the deprecations-for-removal occurred in Java SE 9, before the CSR was active. Instead, those deprecations went through the (Oracle internal) CCC process.
Now that we're fully on the CSR system, I'd expect that deprecations (whether or not for removal) and removals of Java SE APIs would have separate CSR requests. The reason is that adding or changing a deprecation annotation is a spec change, and removing the API is a distinct spec change. They also occur in different releases.
Good points. Though given the annotation is on the method being removed it's really only one spec change. Adding @Deprecated(forRemoval=true) in one release and then removing the element in a later release has to be tracked as two spec changes, and thus two CSRs. There may be more steps of course. In the case of Thread.stop(Throwable) under discussion here, and several SecurityManager methods, the methods were "degraded" (to always throw an exception or only check AllPermission in the case of the SM methods) before removal. So that's another intermediate spec change that has to be tracked via the CSR.
-Alan
Hi Stuart! Nooooooo! Just kidding! Yaaaaaay!!!! Actual removal looks fine but what about the corresponding JDI code: ./jdk.jdi/share/classes/com/sun/jdi/ThreadReference.java it still has a stop(Throwable) function (it doesn't have the no-arg one!). What happens to it? The JDI functions were never deprecated in line with the Thread functions (suspend/resume/stop). The ThreadReference javadoc has: * @see java.lang.Thread#stop(Throwable) so will need fixing. Thanks, David On 2/06/2018 7:16 AM, Stuart Marks wrote:
Hi all,
Please review this changeset to remove the Thread.destroy() and Thread.stop(Throwable) methods. Both of these methods have been deprecated for a long time, and they were deprecated for removal in Java SE 9.
Thread.destroy() was never implemented, and has always thrown NoSuchMethodError back to the caller. Thread.stop(Throwable) was made non-functional in JDK 8, throwing UnsupportedOperationException back to the caller.
Note that the no-arg Thread.stop() method remains deprecated, not for removal, and is unaffected by this changeset.
Note also that ThreadGroup.destroy() doesn't actually destroy any threads -- just thread groups -- and that it is also unaffected by this changeset.
Bug:
https://bugs.openjdk.java.net/browse/JDK-8204243
Webrev:
http://cr.openjdk.java.net/~smarks/reviews/8204243/webrev.0/
Thanks,
s'marks aka @DrDeprecator
On 6/1/18 4:40 PM, David Holmes wrote:
Hi Stuart!
Nooooooo!
Just kidding!
Yaaaaaay!!!!
:-)
Actual removal looks fine but what about the corresponding JDI code:
./jdk.jdi/share/classes/com/sun/jdi/ThreadReference.java
it still has a stop(Throwable) function (it doesn't have the no-arg one!). What happens to it? The JDI functions were never deprecated in line with the Thread functions (suspend/resume/stop).
The ThreadReference javadoc has:
* @see java.lang.Thread#stop(Throwable)
so will need fixing.
Good catch! I had grepped around the source tree for stuff like this but I had missed this one. It looks to me like this interface resides on the debugger side of JDWP. I don't know exactly where it's implemented in the target VM, but I imagine it goes through some VM-internal or JDWP implementation interface, not through the target VM's public java.lang.Thread.stop(thr) interface. Is that right? (I'm not that familiar with this area of the system.) I'm presuming that it doesn't go through j.l.Thread.stop(Throwable) since that method has essentially been non-functional since JDK 8. I see a couple tests for this in test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop and I can see some evidence in our internal build/test systems that they're still being run, so it looks like these interfaces are still active. As you observe, they haven't been deprecated. There might even be valid uses cases for stopping a thread this way in a debugging situation that aren't valid for applications. I guess that means the thing to do is to update the documentation to remove the reference to the soon-to-be-deleted Thread.stop(Throwable) but otherwise leave this stuff unchanged. s'marks
On 2/06/2018 10:43 AM, Stuart Marks wrote:
On 6/1/18 4:40 PM, David Holmes wrote:
Hi Stuart!
Nooooooo!
Just kidding!
Yaaaaaay!!!!
:-)
Actual removal looks fine but what about the corresponding JDI code:
./jdk.jdi/share/classes/com/sun/jdi/ThreadReference.java
it still has a stop(Throwable) function (it doesn't have the no-arg one!). What happens to it? The JDI functions were never deprecated in line with the Thread functions (suspend/resume/stop).
The ThreadReference javadoc has:
* @see java.lang.Thread#stop(Throwable)
so will need fixing.
Good catch! I had grepped around the source tree for stuff like this but I had missed this one.
It looks to me like this interface resides on the debugger side of JDWP. I don't know exactly where it's implemented in the target VM, but I imagine it goes through some VM-internal or JDWP implementation interface, not through the target VM's public java.lang.Thread.stop(thr) interface. Is that right? (I'm not that familiar with this area of the system.)
It's a rather complex setup but the backend that does the work is (usually?) JVM TI, which won't go back out through the Java API.
I'm presuming that it doesn't go through j.l.Thread.stop(Throwable) since that method has essentially been non-functional since JDK 8.
I see a couple tests for this in
test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop
and I can see some evidence in our internal build/test systems that they're still being run, so it looks like these interfaces are still active. As you observe, they haven't been deprecated. There might even be valid uses cases for stopping a thread this way in a debugging situation that aren't valid for applications.
I'm yet to find one :) I pointed out this inconsistency many many years ago but it remains.
I guess that means the thing to do is to update the documentation to remove the reference to the soon-to-be-deleted Thread.stop(Throwable) but otherwise leave this stuff unchanged.
Yep that's what I thought too. Thanks, David
s'marks
On 02/06/2018 06:36, David Holmes wrote:
On 2/06/2018 10:43 AM, Stuart Marks wrote:
:
It looks to me like this interface resides on the debugger side of JDWP. I don't know exactly where it's implemented in the target VM, but I imagine it goes through some VM-internal or JDWP implementation interface, not through the target VM's public java.lang.Thread.stop(thr) interface. Is that right? (I'm not that familiar with this area of the system.)
It's a rather complex setup but the backend that does the work is (usually?) JVM TI, which won't go back out through the Java API. Right, there's a StopThread function in JVM TI and a corresponding Stop Command in the JDWP protocol. The JVM TI spec has a statement to say that it works "similar to Thread.stop" and the JDWP spec has "as if done by Thread.stop" so it would be good to update both of these.
-Alan
On 2/06/2018 4:38 PM, Alan Bateman wrote:
On 02/06/2018 06:36, David Holmes wrote:
On 2/06/2018 10:43 AM, Stuart Marks wrote:
:
It looks to me like this interface resides on the debugger side of JDWP. I don't know exactly where it's implemented in the target VM, but I imagine it goes through some VM-internal or JDWP implementation interface, not through the target VM's public java.lang.Thread.stop(thr) interface. Is that right? (I'm not that familiar with this area of the system.)
It's a rather complex setup but the backend that does the work is (usually?) JVM TI, which won't go back out through the Java API. Right, there's a StopThread function in JVM TI and a corresponding Stop Command in the JDWP protocol. The JVM TI spec has a statement to say that it works "similar to Thread.stop" and the JDWP spec has "as if done by Thread.stop" so it would be good to update both of these.
Any suggestions as to how to do that in a practical and effective way? "As if done by the highly-dangerous, long-deprecated and finally removed Thread.stop(Throwable)" ? ;-) In all seriousness I hate to do anything that might suggest these are valid API's even for tools. Maybe we should deprecate them as well (separate RFE of course). David
-Alan
On 03/06/2018 13:11, David Holmes wrote:
Any suggestions as to how to do that in a practical and effective way?
"As if done by the highly-dangerous, long-deprecated and finally removed Thread.stop(Throwable)"
? ;-)
In all seriousness I hate to do anything that might suggest these are valid API's even for tools. Maybe we should deprecate them as well (separate RFE of course).
These date back to JPDA in JDK 1.2 (it was JVMDI at the time, pre-dates JVM TI) and would need research to see if there are debuggers or maybe fault injection tools are using it. For Stuart's current effort then it will need a bit of text, maybe borrowing old spec where the operation was specified to "stop" a thread with an asynchronous exception. Warnings about the dangers are probably appropriate but these are of course interfaces that developers are unlikely to ever use directly. -Alan
On 6/3/18 8:55 AM, Alan Bateman wrote:
On 03/06/2018 13:11, David Holmes wrote:
Any suggestions as to how to do that in a practical and effective way?
"As if done by the highly-dangerous, long-deprecated and finally removed Thread.stop(Throwable)"
? ;-)
In all seriousness I hate to do anything that might suggest these are valid API's even for tools. Maybe we should deprecate them as well (separate RFE of course).
These date back to JPDA in JDK 1.2 (it was JVMDI at the time, pre-dates JVM TI) and would need research to see if there are debuggers or maybe fault injection tools are using it.
For Stuart's current effort then it will need a bit of text, maybe borrowing old spec where the operation was specified to "stop" a thread with an asynchronous exception. Warnings about the dangers are probably appropriate but these are of course interfaces that developers are unlikely to ever use directly.
The spec for com.sun.jdk.ThreadReference.stop(Throwable) seems sufficiently descriptive to stand alone if the @see Thread.stop(Throwable) cross reference is simply removed. Are there any other changes that need to be done as part of this changeset? s'marks
On 05/06/2018 01:09, Stuart Marks wrote:
:
The spec for com.sun.jdk.ThreadReference.stop(Throwable) seems sufficiently descriptive to stand alone if the @see Thread.stop(Throwable) cross reference is simply removed.
Are there any other changes that need to be done as part of this changeset? The source file that is used to generate the JDWP protocol code and the spec is in make/data/jdwp/jdwp.spec. The JVM TI spec is at src/hotspot/share/prims/jvmti.xml. It should be okay to skip those for this change-set, assuming it is followed up quickly with another change-set to update those specs.
-Alan
[adding serviceability-dev] Hi serviceability folks, I'm in the process of removing Thread.destroy() and Thread.stop(Throwable) from the Java SE API. Alan and David have pointed out that there are some cross-references to Thread.stop(Throwable) in the JDWP and JVMTI specs, as well as in the JDI ThreadReference API. I've adjusted the relevant files. See please review this updated webrev: http://cr.openjdk.java.net/~smarks/reviews/8204243/webrev.1/ For context, please see the full review thread on core-libs-dev: http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-June/053536.html On 6/4/18 11:11 PM, Alan Bateman wrote:
The source file that is used to generate the JDWP protocol code and the spec is in make/data/jdwp/jdwp.spec. The JVM TI spec is at src/hotspot/share/prims/jvmti.xml. It should be okay to skip those for this change-set, assuming it is followed up quickly with another change-set to update those specs.
OK. I took a look at those other files and they seem simple enough to update as part of this changeset. If there aren't any objections from anyone, might as well get this all done at once. Thanks, s'marks
Hi Stuart, This all looks fine to me. One minor nit in threadPrimitiveDeprecation.html is whether the new statements you reference a particular Java version? It reads a little oddly to go through all the explanation and then say "Thread.x has been removed". In the spirit of @since you might say "has been removed as of JDK 11". Or arguably you could remove discussion of the removed methods altogether. Thanks, David On 6/06/2018 10:05 AM, Stuart Marks wrote:
[adding serviceability-dev]
Hi serviceability folks,
I'm in the process of removing Thread.destroy() and Thread.stop(Throwable) from the Java SE API. Alan and David have pointed out that there are some cross-references to Thread.stop(Throwable) in the JDWP and JVMTI specs, as well as in the JDI ThreadReference API. I've adjusted the relevant files.
See please review this updated webrev:
http://cr.openjdk.java.net/~smarks/reviews/8204243/webrev.1/
For context, please see the full review thread on core-libs-dev:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-June/053536.html
On 6/4/18 11:11 PM, Alan Bateman wrote:
The source file that is used to generate the JDWP protocol code and the spec is in make/data/jdwp/jdwp.spec. The JVM TI spec is at src/hotspot/share/prims/jvmti.xml. It should be okay to skip those for this change-set, assuming it is followed up quickly with another change-set to update those specs.
OK. I took a look at those other files and they seem simple enough to update as part of this changeset. If there aren't any objections from anyone, might as well get this all done at once.
Thanks,
s'marks
On 06/06/2018 01:05, Stuart Marks wrote:
[adding serviceability-dev]
Hi serviceability folks,
I'm in the process of removing Thread.destroy() and Thread.stop(Throwable) from the Java SE API. Alan and David have pointed out that there are some cross-references to Thread.stop(Throwable) in the JDWP and JVMTI specs, as well as in the JDI ThreadReference API. I've adjusted the relevant files.
See please review this updated webrev:
http://cr.openjdk.java.net/~smarks/reviews/8204243/webrev.1/ Have you considered removing the "What about Thread.stop(Throwable)" section completely from the threadPrimitiveDeprecation doc? A new developer reading the javadoc in 2019 shouldn't need to read about a removed method.
Otherwise I think this looks good, including the updates to the JDWP and JVM TI specs. -Alan
Hi, Stuart. I think you need to make changes to this file too: http://hg.openjdk.java.net/jdk/jdk/file/tip/src/java.base/share/classes/java... Thanks, iris -----Original Message----- From: Alan Bateman Sent: Wednesday, June 6, 2018 3:40 AM To: Stuart Marks <stuart.marks@oracle.com>; serviceability-dev <serviceability-dev@openjdk.java.net> Cc: core-libs-dev <core-libs-dev@openjdk.java.net> Subject: Re: RFR(s): 8204243: remove Thread.destroy() and Thread.stop(Throwable) On 06/06/2018 01:05, Stuart Marks wrote:
[adding serviceability-dev]
Hi serviceability folks,
I'm in the process of removing Thread.destroy() and Thread.stop(Throwable) from the Java SE API. Alan and David have pointed out that there are some cross-references to Thread.stop(Throwable) in the JDWP and JVMTI specs, as well as in the JDI ThreadReference API. I've adjusted the relevant files.
See please review this updated webrev:
http://cr.openjdk.java.net/~smarks/reviews/8204243/webrev.1/ Have you considered removing the "What about Thread.stop(Throwable)" section completely from the threadPrimitiveDeprecation doc? A new developer reading the javadoc in 2019 shouldn't need to read about a removed method.
Otherwise I think this looks good, including the updates to the JDWP and JVM TI specs. -Alan
Yeah, maybe it's better simply to remove the mentions of the methods that are being removed. I'll do so. I note that there are many other updates that could be done (the examples use applets!) but I think that's a task for another time. s'marks On 6/6/18 9:28 AM, Iris Clark wrote:
Hi, Stuart.
I think you need to make changes to this file too:
http://hg.openjdk.java.net/jdk/jdk/file/tip/src/java.base/share/classes/java...
Thanks, iris
-----Original Message----- From: Alan Bateman Sent: Wednesday, June 6, 2018 3:40 AM To: Stuart Marks <stuart.marks@oracle.com>; serviceability-dev <serviceability-dev@openjdk.java.net> Cc: core-libs-dev <core-libs-dev@openjdk.java.net> Subject: Re: RFR(s): 8204243: remove Thread.destroy() and Thread.stop(Throwable)
On 06/06/2018 01:05, Stuart Marks wrote:
[adding serviceability-dev]
Hi serviceability folks,
I'm in the process of removing Thread.destroy() and Thread.stop(Throwable) from the Java SE API. Alan and David have pointed out that there are some cross-references to Thread.stop(Throwable) in the JDWP and JVMTI specs, as well as in the JDI ThreadReference API. I've adjusted the relevant files.
See please review this updated webrev:
http://cr.openjdk.java.net/~smarks/reviews/8204243/webrev.1/ Have you considered removing the "What about Thread.stop(Throwable)" section completely from the threadPrimitiveDeprecation doc? A new developer reading the javadoc in 2019 shouldn't need to read about a removed method.
Otherwise I think this looks good, including the updates to the JDWP and JVM TI specs.
-Alan
participants (5)
-
Alan Bateman
-
David Holmes
-
Iris Clark
-
Lance Andersen
-
Stuart Marks