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

[David Holmes]

Hi Robbin,

On 10/06/2020 5:55 pm, Robbin Ehn wrote:
> 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?

Honestly I think I'd like to see things reverted to the use of 
calling_thread as done for the VMOperation previously. We know it is 
functionally correct and it should also have the same performance profile.

Thanks,
David

> 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