RFR: 8200195 : serviceability/jvmti/FieldAccessWatch/FieldAccessWatch.java crashes with "assert(thread->thread_state() == _thread_in_native) failed: coming from wrong thread state"

[Chris Plummer]

On 4/9/18 3:04 PM, Alex Menkov wrote:
> updated webrev: 
> http://cr.openjdk.java.net/~amenkov/field_access_graal/webrev.02/
> (javaEnv is replaced with jni_env)
Looks good.
>
> On 04/09/2018 14:35, David Holmes wrote:
>> +1
>>
>> Though I'd also like to understand why the test only fails sometimes 
>> and which other thread is involved?
>
> It always fails with Graal compiler on win, linux and macosx.
> Looks like with Graal compiler access/modification events are sent on 
> several threads (sorry, I don't have enough knowledge about Graal).
>
One thing to be careful of is that testResultClass and testResultObject 
may not be valid in the thread that is doing the GetFieldID() call. 
These were setup in Java_FieldAccessWatch_startTest() which might have 
been a different thread, with a different ClassLoader hierarchy, than 
the callback thread. I ran into this problem in another test where there 
was an unexpected callback from a finalizer thread. Probably in this 
case the worse that can happen is an exception, and the test deals with 
that.

Chris
> --alex
>
>>
>> Thanks,
>> David
>>
>> On 10/04/2018 7:28 AM, Chris Plummer wrote:
>>> Hi Alex,
>>>
>>> I'd suggest renaming javaEnv to jni_env to be consistent. Not sure 
>>> why javaEnv was chosen in the original implementation. Otherwise the 
>>> changes look good.
>>>
>>> thanks,
>>>
>>> Chris
>>>
>>> On 4/9/18 2:13 PM, Alex Menkov wrote:
>>>> Hi all,
>>>>
>>>> Please review a fix for 
>>>> https://bugs.openjdk.java.net/browse/JDK-8200195
>>>> webrev: http://cr.openjdk.java.net/~amenkov/field_access_graal/webrev/
>>>>
>>>> The problem with the test is it uses cached JNIEnv value instead 
>>>> using a value passed to the callbacks. JNIEnv is valid only for the 
>>>> current thread.
>>>>
>>>> --alex
>>>
>>>