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



Reply via email to