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

[Serguei Spitsyn]

On Tue, 21 Oct 2025 20:32:12 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 three additional commits since the last revision:
> 
>  - completed renaming
>  - renamed test
>  - updated after feedback

Thank you for the update! It looks pretty good to me.
I've posted several more suggestions though.

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

> 2281:   // In this case the frame is only marked for deoptimization but still remains compiled.
> 2282:   // Also, the last frame might be compiled if events were not enabled for
> 2283:   // this thread. The thread filtering is done later.

Q: The following part of comment is confusing: `" ... if events were not enabled for this thread."
     How do we post the events if they were not enabled? Do I miss anything?

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

> 28:  * @run main/othervm/native -agentlib:JvmtiFieldEventsFromJNI FieldsEventsFromJNI
> 29:  */
> 30: public class FieldsEventsFromJNI {

Nit: The class name still does not match the test folder name: `s/FieldsEventsFromJNI/FieldEventsFromJNI/g`

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

> 42:   }
> 43:   deallocate(jvmti,jni, m_name);
> 44: 

Nit: Still unneeded empty line. :)

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

> 165: }
> 166: 
> 167: 

Nit: Still unneeded empty line. :)

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

> 331:   jvmtiError err = jvmti->GetFieldName(field_class, field, &name, &signature, nullptr);
> 332:   check_jvmti_status(jni, err, "get_field_name: error in JVMTI GetFieldName call");
> 333:   deallocate(jvmti, jni, signature);

Nit: Sorry, I've not noticed that the signature is not really needed. Then `nullptr` can be passed instead of `&signature` as well.

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

PR Review: https://git.openjdk.org/jdk/pull/27584#pullrequestreview-3363022782
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2449945745
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2449929078
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2449926090
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2449926933
PR Review Comment: https://git.openjdk.org/jdk/pull/27584#discussion_r2449924748