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

[Robbin Ehn]

Hi Martin,

On 2019-12-16 16:02, Doerr, Martin wrote:
> Hi Robbin,
> 
> thanks for your explanations.

Thanks for looking at this!

> 
> The InlineCacheBuffer is tricky, but looks correct to me. I wonder if we couldn't use set_pending_async_exception instead of the send_async_exception call. But that's not necessarily subject of this change.
> 
> I think the name ThreadStopClosure is misleading. It doesn't stop any thread. It installs an async exception. Would you mind giving it a more accurate name?
> What about InstallAsyncExceptionClosure?

Yes, sure.

> 
> Looks good to me otherwise.

Thanks, Robbin

> 
> Best regards,
> Martin
> 
> 
>> -----Original Message-----
>> From: hotspot-runtime-dev <hotspot-runtime-dev-
>> bounces at openjdk.java.net> On Behalf Of Robbin Ehn
>> Sent: Montag, 16. Dezember 2019 14:28
>> To: David Holmes <david.holmes at oracle.com>; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: Re: RFR(s): 8235913: ThreadStop should be a handshake
>>
>> Hi David,
>>
>> On 12/16/19 12:28 PM, David Holmes wrote:
>>> Hi Robbin,
>>>
>>> On 16/12/2019 7:51 pm, Robbin Ehn wrote:
>>>> Hi all, please review:
>>>>
>>>> Making VM_ThreadStop into a handshake instead.
>>>>
>>>> Issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8235913
>>>>
>>>> Code:
>>>> http://cr.openjdk.java.net/~rehn/8235913/v1/webrev/index.html
>>>
>>> It isn't obvious to me that the code in send_thread_stop that was
>> previously
>>> always executed by the VMThread can be safely executed by the target
>> JavaThread
>>> itself. In particular when executing InlineCacheBuffer::refill_ic_stubs()
>> what
>>> state is the JavaThread in?
>>
>> To run VMThread::execute() you must be in _thread_in_vm. (we assert this)
>> I even think send_thread_stop is less error-prone when 'this' is the same as
>> Thread::current().
>>
>>>
>>> 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)
>> And test like StopAtExit.java which do the java version.
>>
>> Thanks, Robbin
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> Passes t1-7.
>>>>
>>>> Thanks, Robbin