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

[Serguei Spitsyn]

On Wed, 22 Oct 2025 05:28:37 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:
> 
>   the updated comments

The fix looks pretty good. I've posted several nits though.

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

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

Nit: Better to give the local some meaningful name instead of `c`. :)

test/hotspot/jtreg/serviceability/jvmti/events/FieldAccess/FieldsEventsFromJNI/FieldsEventsFromJNI.java line 62:

> 60:         synchronized(lock) {
> 61:             anotherThread.start();
> 62:             while(!isAnotherThreadStarted) {

Nit: Missed space after `while` and `synchronized` at lines: 53, 56, 60 and 62.

test/hotspot/jtreg/serviceability/jvmti/events/FieldAccess/FieldsEventsFromJNI/libFieldsEventsFromJNI.cpp line 135:

> 133:     fatal(jni, "No class found");
> 134:   }
> 135:   jfieldID fieldToRead = jni->GetFieldID(cls, "accessField", "Ljava/lang/String;");

Nit: It is better to define a constant for the field name `"accessField"`. Also it used in two places at lines: 52 and 135. The same is true for the `"SetFieldAccessWatch"`.

test/hotspot/jtreg/serviceability/jvmti/events/FieldAccess/FieldsEventsFromJNI/libFieldsEventsFromJNI.cpp line 193:

> 191:   check_jvmti_error(err, "SetEventNotificationMode");
> 192: 
> 193:   if (modify_cnt != isEventExpected) {

Nit: Comparing an `int` with a jboolean`. The same is at line 160. I'd suggest to pass expected number of events instead of jboolean. Then it is better to define `access_cnt` and `modify_cnt` as `jint`.

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

PR Review: https://git.openjdk.org/jdk/pull/27584#pullrequestreview-3375321998
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2459466954
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2459470649
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2459533264
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2459543963