RFR(s): 8235913: ThreadStop should be a handshake

[David Holmes]

Incremental changes look good. I guess we will see how this works out. :)

Thanks,
David

On 17/12/2019 11:57 pm, Robbin Ehn wrote:
> Hi David,
> 
> On 12/17/19 12:39 AM, David Holmes wrote:
>>> I even think send_thread_stop is less error-prone when 'this' is the 
>>> same as Thread::current().
>>
>> I remain concerned when we suddenly will start executing code in 
>> threads that have never before executed this code.
> 
> This code is executed when running nsk_jvmti.
> 
>>
>>>>
>>>> Do we actually have tests that exercise that particular code path? 
>>>> I'd also check that we test JVM TI stopping of the current thread. 
>>>> (JVM_ThreadStop is not an issue as it already special cases the 
>>>> current thread).
>>>
>>> We have several test that do:
>>> jvmti->StopThread(thread, exception)
>>
>> Yes but the issue is with stopping the current thread.
>>
>> I'm mostly concerned that the refill_ic_stubs() code path may not be 
>> getting tested, or even executed at all given how rarely async 
>> exceptions are used in practice.
> 
> If I run nsk_jvmti I hit it like 30 times.
> 
>>
>> When a thread executes:
>>
>> Handshake::execute(&vm_stop, target);
>>
>> does that perform any thread-state transitions or related checks that 
>> would install the pending-async-exception as an actual pending 
>> exception? If so that breaks the refill_ic_stubs code.
> 
> Both VMThread::execute() and Handshake::execute() adds a VM_op on to 
> VMThread queue and current thread goes to blocked.
> If there is an issue, it should already be present.
> But I think you are correct that there is an issue.
> And I think Martin was correct, we should just re-installed it with:
> set_pending_async_exception
> 
> It very hard to hit code path, getting an exception in the safepoint for 
> ic stub
> refill.
> 
> Inc:
> http://cr.openjdk.java.net/~rehn/8235913/v2/inc/webrev/
> Full:
> http://cr.openjdk.java.net/~rehn/8235913/v2/full/webrev/
> 
> Thanks, Robbin
> 
>>
>> Thanks,
>> David
>>
>>> And test like StopAtExit.java which do the java version.
>>>
>>> Thanks, Robbin
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Passes t1-7.
>>>>>
>>>>> Thanks, Robbin