RFR: 8224852: JVM crash on watched field access from native code [v2]

[Serguei Spitsyn]

On Sat, 18 Oct 2025 17:24:43 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> The field access/modification events set interp only mode and compiled frame is not expected. However JNI might call `post_field_access_by_jni` while the last java frame is compiled. 
>> 
>> 1) The thread switched to interponly mode while it is in JNI code. The deoptimization is triggered but each frame is really changed only execution returns to it.  So last java frame was not executed and thus is still compiled. 
>> 2) The JNI accessed field from the thread where field events are not enabled. So the `post_field_access_by_jni` is called in threads in interp_only mode. 
>> 
>> The original example doesn't reproduce issue because of JDK changes and I don't know of it is 1) or 2)I. I implemented regression test for both problems. 
>> 
>> The location should be zero for JNI access.
>
> Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed comment

src/hotspot/share/prims/jvmtiExport.cpp line 2226:

> 2224:     javaVFrame *jvf = thread->last_java_vframe(&reg_map);
> 2225:     method = jvf->method();
> 2226:     address = jvf->method()->code_base();

Nit: I'm thinking if this code can be simplified a little bit if the same approach with `reg_map` can be used for interpreter frame as well.

src/hotspot/share/prims/jvmtiExport.cpp line 2332:

> 2330:     javaVFrame *jvf = thread->last_java_vframe(&reg_map);
> 2331:     method = jvf->method();
> 2332:     address = jvf->method()->code_base();

Nit: Same question as above on a potential simplification by unifying the interpreter and compile frame cases.

test/hotspot/jtreg/serviceability/jvmti/events/FieldAccess/FieldEventsFromJNI/TestFieldsEventsFromJNI.java line 29:

> 27:  * @bug 8224852
> 28:  * @run main/othervm/native -agentlib:JvmtiFieldEventsFromJNI TestFieldsEventsFromJNI
> 29:  * @run main/othervm/native -agentlib:JvmtiFieldEventsFromJNI -Xcomp TestFieldsEventsFromJNI

Nit: It is convenient when the test folder and the Java file have the same name. Many tests in the `serviceability/jvmti` folder follow this rule. So, I'd suggest to get rid of the `Test` prefix in this class name. An additional advantage would be that the class name is shorter. :)

test/hotspot/jtreg/serviceability/jvmti/events/FieldAccess/FieldEventsFromJNI/TestFieldsEventsFromJNI.java line 48:

> 46: 
> 47:     public static void main(String[] args) throws InterruptedException {
> 48:         System.loadLibrary("JvmtiFieldEventsFromJNI");

Nit: Is this really needed? In fact, we have it in many tests though.

test/hotspot/jtreg/serviceability/jvmti/events/FieldAccess/FieldEventsFromJNI/libJvmtiFieldEventsFromJNI.cpp line 43:

> 41:   jni->DeleteLocalRef(object_class);
> 42:   return obj_class_name;
> 43: }

Nit: Would it be good to add this into `jvmti_common.hpp`?

test/hotspot/jtreg/serviceability/jvmti/events/FieldAccess/FieldEventsFromJNI/libJvmtiFieldEventsFromJNI.cpp line 102:

> 100:   deallocate(jvmti,jni, f_name);
> 101: 
> 102: 

Nit: Unneeded empty lines: 68, 102 and 137.

test/lib/jdk/test/lib/jvmti/jvmti_common.hpp line 335:

> 333:   check_jvmti_status(jni, err, "get_field_name: error in JVMTI GetFieldName call");
> 334:   deallocate(jvmti,jni, signature);
> 335:   deallocate(jvmti,jni, generic);

Nit: The lines 334 and 335 miss space after comma in the argument lists. Also, it is better to pass `nullptr` instead of `&generic`, so there would not be need to have this local and to deallocate the returned string.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2447046252
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2447054396
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2447066803
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2447075224
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2447101802
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2447108290
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2447094326