RFR(s): 8247248: JVM TI might create JNI locals in another thread when using handshakes.

[Robbin Ehn]

Hi David,

On 2020-06-10 04:30, David Holmes wrote:
> Hi Robbin,
> 
> On 10/06/2020 2:15 am, Robbin Ehn wrote:
>> Hi all,
>>
>> If the direct handshake is executed by the target thread, the JNI
>> local(s) are created in that thread but returned in the handshaking
>> thread.
>> They thus are not safe to use. (thread might even have exited by this
>> point)
>>
>> Code:
>> http://cr.openjdk.java.net/~rehn/8247248/v1/webrev/
>>
>> Unfortunately there is no way the distinguish a local jobject vs a
>> global. Which makes it hard to track when the jobject is global and not.
> 
> I have some comments/concerns that I've added to the bug report.
> 
> Switching from local refs to global refs adds a bit of overhead and will 
> likely impact the performance here. Not a showstopper but would be nice 
> to avoid if possible as it seems a bit of a kludge to communicate values 
> across threads via global refs.

Using our test to measure nanos, it's seem to be 5%, 21 us to 22 us when 
target is current thread, no handshake at all.
(handshake case is around 65 us, so noise is larger than overhead).

In earlier version of the patch I skipped global when doing thread self.
But since it's not easy to track if it's a global or local I removed 
that for the simplification.

> 
> In the old VMOperation solution the VMThread used the JvmtiEnv* of the 
> calling thread so that the local refs were in the right place. Can we do 
> the same with the handshake code? It will mean restoring the "calling 
> thread" argument to the handshake operation but I think this is a 
> workable approach. (The removal of the calling thread argument should 
> have rung alarm bells :( .)
> 
> Or, could this be case where we don't want the target thread to execute 
> the handshake and we need a way to mark the handshake operation as such? 
> That's a bigger change of course.

This is a good idea, and we can easily implement a generic facility in 
the, hopefully, upcoming asynchronous handshakes.

But no matter in what form I don't see us getting this enhancement to 
handshakes before JDK 15.

How do you want to proceed for JDK 15?

Thanks, Robbin

> 
> Thanks,
> David
> -----
> 
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8247248
>>
>> Local testing of JDI/JVMTI and t1-5.
>> (no real crash so there is nothing to reproduce)
>>
>> Thanks, Robbin