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

[Doerr, Martin]

Hi Robbin,

thanks for your explanations.

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?

Looks good to me otherwise.

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