RFR(XS): 8234882: JVM TI StopThread should only allow ThreadDeath

[David Holmes]

On 27/05/2020 6:36 pm, serguei.spitsyn at oracle.com wrote:
> Hi David,
> 
> 
> On 5/27/20 00:47, David Holmes wrote:
>> Hi Serguei,
>>
>> On 27/05/2020 1:01 pm, serguei.spitsyn at oracle.com wrote:
>>> Please, review a fix for:
>>>    https://bugs.openjdk.java.net/browse/JDK-8234882
>>>
>>> CSR draft (one CSR reviewer is needed before finalizing it):
>>>    https://bugs.openjdk.java.net/browse/JDK-8245853
>>
>> I have some thoughts on the wording which I will add to the CSR.
> 
> Thank you a lot for looking at this!
> 
>> Also on reflection I think JVMTI_ERROR_ILLEGAL_ARGUMENT would the best 
>> error to use, and it has an equivalent in JDWP and at the Java level 
>> for JDI.
> 
> This is an interesting variant, thanks!
> We need to balance on several criteria:
>   1) Compatibility: keep returning error as close as possible to the 
> current spec

If you are adding a new error condition I don't understand what you mean 
by "close to the current spec" ??

>   2) Best error naming match between JVM TI and JDI/JDWP
>   3) Best practice in errors naming

If the argument is not a ThreadDeath instance then it is an illegal 
argument - perfect fit semantically all the specs involved have an 
"illegal argument" error form.

Cheers,
David

> I think the #1 is most important but will look at it once more.
> 
> Thanks,
> Serguei
> 
>> Thanks,
>> David
>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/jvmti-stop-thread.1/src/ 
>>>
>>>
>>> Updated JVM TI StopThread spec:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/jvmti-stop-thread.1/docs/specs/jvmti.html#StopThread 
>>>
>>>
>>> Summary:
>>>
>>>    The JVM TI StopThread method mirrored the functionality of the
>>>    java.lang.Thread::stop(Throwable t) method, in that it allows any 
>>> exception
>>>    type to be installed as an asynchronous exception in the target 
>>> thread.
>>>    However, the java.lang.Thread::stop(Throwable t) method was 
>>> inherently unsafe
>>>    and in Java 8 (under JDK-7059085) it was "retired" so that it 
>>> always threw
>>>    UnsupportedOperationException.
>>>    The updated JVM TI StopThread spec disallows an arbitrary 
>>> Throwable from being passed,
>>>    and instead restricts the argument to being an instance of 
>>> ThreadDeath, thus
>>>    mirroring the (deprecated but still functional) 
>>> java.lang.Thread::stop() method.
>>>    The error JVMTI_ERROR_INVALID_OBJECT is returned if the exception 
>>> argument
>>>    is not an instance of ThreadDeath.
>>>
>>>    Also, I will file similar RFE and CSR on the JDI and JDWP spec.
>>>
>>>
>>> Testing:
>>>    Built docs and checked the doc has been generated as expected.
>>>    Will run the nsk.jvmti tests locally.
>>>    Will submit hs-tiers1-3 to make sure there are no regressions in 
>>> the JVM TI and JDI tests.
>>>
>>> Thanks,
>>> Serguei
>