8316439: Several jvmti tests fail with -Xcheck:jni

[shivshah-oracle]

Fixed three test files that fail with -Xcheck:jni on debug builds:

popframe007.cpp: CallStaticVoidMethod in native B() was called with the declaring class (popframe007) instead of TestThread where the method ID belongs. Stored TestThread class as a global ref in getReady() and used it in B()

aod.cpp: nsk_aod_agentLoaded() and nsk_aod_agentFinished() call CallStaticVoidMethod without checking for pending exceptions. Added nsk_aod_checkJNIException wrapper that describes, clears, and fails on unexpected exceptions. Fixes all 13 AttachOnDemand tests

retransform003.cpp: ClassFileLoadHook callback calls CallObjectMethod and CallStaticObjectMethod without checking for pending exceptions. Added ExceptionCheck/ExceptionClear after both calls

bi02t001 and bi03t001 pass without changes, NMT corruption issue from JDK 22 appears resolved on JDK 27

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8316439: Several jvmti tests fail with -Xcheck:jni (Bug - P4)

Reviewers

• Chris Plummer (@plummercj - Reviewer)
• Ekaterina Pavlova (@katyapav - Committer)
• Leonid Mesnik (@lmesnik - Reviewer)
• Serguei Spitsyn (@sspitsyn - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30711/head:pull/30711
$ git checkout pull/30711

Update a local copy of the PR:
$ git checkout pull/30711
$ git pull https://git.openjdk.org/jdk.git pull/30711/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 30711

View PR using the GUI difftool:
$ git pr show -t 30711

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30711.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back shivshah-oracle! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@shivshah-oracle This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8316439: Several jvmti tests fail with -Xcheck:jni

Reviewed-by: cjplummer, epavlova, lmesnik, sspitsyn

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 219 new commits pushed to the master branch:

• 44c484c: 8381677: Remove dependence of UseSHA on UseSHAIntrinsics
• 49a15be: 8382020: Time Zone Abbreviation Not Localized for Non-English Locales
• d4bb97c: 8379677: G1: Micros-System.gc benchmarks regress after moving to PartialArrayStates
• ... and 216 more: https://git.openjdk.org/jdk/compare/91892948ee0ac3c62bbddbbf9e6f99e746f3d72f...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@plummercj, @katyapav, @lmesnik, @dholmes-ora, @sspitsyn) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk]

@shivshah-oracle The following label will be automatically applied to this pull request:

• serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (28000a06)

[plummercj]

Did this just end up getting an exception that we never handle anywhere?

[shivshah-oracle]

On debug builds with -Xcheck:jni, it catches the class/method ID mismatch and aborts with a fatal error before the call even executes. Without -Xcheck:jni, the behavior is undefined per the JNI spec. It could silently leave a pending exception or produce incorrect results.

[katyapav]

The fix looks good, all nsk/jvmti and nsk/aod tests passed on linux, windows and macos, approve.

[dholmes-ora]

If this code returns to Java (I can't tell easily) then we should not clear the exception. Otherwise we should use ExceptionDescribe as you do below so that we know that something has gone wrong in the test.

[sspitsyn]

If this code returns to Java (I can't tell easily) then we should not clear the exception.

It is in the ClassFileLoadHook callback (posting a CFLH event), so this code should eventually return to Java. Was the original error reported by -Xcheck:jni that the exception has not been cleared?

Update:
This is an assert:

 # assert(!_thread->is_pending_jni_exception_check()) failed: Pending JNI Exception Check

class ThreadToNativeFromVM : public ThreadStateTransition {
 public:
  ThreadToNativeFromVM(JavaThread *thread) : ThreadStateTransition(thread) {
    assert(!thread->owns_locks(), "must release all locks when leaving VM");
    transition_from_vm(thread, _thread_in_native);
  }
  ~ThreadToNativeFromVM() {
    transition_from_native(_thread, _thread_in_vm);
    assert(!_thread->is_pending_jni_exception_check(), "Pending JNI Exception Check");  **<== HERE!!!**
    // We don't need to clear_walkable because it will happen automagically when we return to java
  }
};

So, the transition is from native to VM first. It is where we do not except pending exceptions with -Xcheck:jni. So, we probably should use ExceptionDescribe as you suggested.

[dholmes-ora]

Ditto

[sspitsyn]

Looks good modulo a suggestion from David.