8365192: post_meth_exit should be in vm state when calling get_jvmti_thread_state

src/hotspot/share/prims/jvmtiExport.cpp

@@ -420,6 +420,7 @@ JvmtiExport::get_jvmti_interface(JavaVM *jvm, void **penv, jint version) {
 JvmtiThreadState*
 JvmtiExport::get_jvmti_thread_state(JavaThread *thread, bool allow_suspend) {
   assert(thread == JavaThread::current(), "must be current thread");
+  assert(thread->thread_state() == _thread_in_vm, "thread should be in vm");
   if (thread->is_vthread_mounted() && thread->jvmti_thread_state() == nullptr) {
     JvmtiEventController::thread_started(thread);
     if (allow_suspend && thread->is_suspended()) {
@@ -1830,49 +1831,46 @@ void JvmtiExport::post_method_entry(JavaThread *thread, Method* method, frame cu
 void JvmtiExport::post_method_exit(JavaThread* thread, Method* method, frame current_frame) {
   HandleMark hm(thread);
   methodHandle mh(thread, method);
+  oop oop_result;
+  Handle result;
+  jvalue value;
+  value.l = 0L;
+  // post_method_exist is called only when the method is not exit because of
+  // exception so result should be always initialized.
+  // At this point we only have the address of a "raw result" and
+  // we just call into the interpreter to convert this into a jvalue.
+  // Additionally, the result oop should be preserved while the thread is in java.
+  BasicType type = current_frame.interpreter_frame_result(&oop_result, &value);
 
-  JvmtiThreadState *state = get_jvmti_thread_state(thread);
+  if (is_reference_type(type)) {
+    result = Handle(thread, oop_result);
+  }
+  // Deferred transition to VM, so we can stash away the return oop before GC
+  // Note that this transition is not needed when throwing an exception, because
+  // there is no oop to retain.
+  JvmtiThreadState *state;
+  {
+    ThreadInVMfromJava tiv(thread);
+    state = get_jvmti_thread_state(thread);
+  }
 
   if (state == nullptr || !state->is_interp_only_mode()) {
     // for any thread that actually wants method exit, interp_only_mode is set
     return;
   }
-
-  // return a flag when a method terminates by throwing an exception
-  // i.e. if an exception is thrown and it's not caught by the current method
-  bool exception_exit = state->is_exception_detected() && !state->is_exception_caught();
-  Handle result;
-  jvalue value;
-  value.j = 0L;
-
-  if (state->is_enabled(JVMTI_EVENT_METHOD_EXIT)) {
-    // if the method hasn't been popped because of an exception then we populate
-    // the return_value parameter for the callback. At this point we only have
-    // the address of a "raw result" and we just call into the interpreter to
-    // convert this into a jvalue.
-    if (!exception_exit) {
-      oop oop_result;
-      BasicType type = current_frame.interpreter_frame_result(&oop_result, &value);
-      if (is_reference_type(type)) {
-        result = Handle(thread, oop_result);
-        value.l = JNIHandles::make_local(thread, result());
-      }
-    }
+  if (state->is_enabled(JVMTI_EVENT_METHOD_EXIT) && is_reference_type(type)) {
+    value.l = JNIHandles::make_local(thread, result());
   }
-
   // Do not allow NotifyFramePop to add new FramePop event request at
   // depth 0 as it is already late in the method exiting dance.
   state->set_top_frame_is_exiting();
 
-  // Deferred transition to VM, so we can stash away the return oop before GC
-  // Note that this transition is not needed when throwing an exception, because
-  // there is no oop to retain.
   JavaThread* current = thread; // for JRT_BLOCK
   JRT_BLOCK
-    post_method_exit_inner(thread, mh, state, exception_exit, current_frame, value);
+    post_method_exit_inner(thread, mh, state,false, current_frame, value);
   JRT_BLOCK_END
 
-  // The JRT_BLOCK_END can safepoint in ThreadInVMfromJava desctructor. Now it is safe to allow
+  // The JRT_BLOCK_END can safepoint in ThreadInVMfromJava destructor. Now it is safe to allow
   // adding FramePop event requests as no safepoint can happen before removing activation.
   state->clr_top_frame_is_exiting();
 

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/ExceptionOccurred.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @summary Test verifies that MethodExit event is correctly posted
+ * if exception occured in the current thread.
+ *
+ * @bug 8365192
+ * @run main/othervm/native -agentlib:ExceptionOccurred ExceptionOccurred
+ */
+public class ExceptionOccurred {
+
+    private static native void enable();
+    private static native void disableAndCheck();
+
+    static String exceptionExit() {
+        throw new RuntimeException("MyRuntimeException");
+    }
+
+
+    // Called from ExcptionExit MethodExit callback via JNI
+    static String upCall() {
+        return "MyNewString";
+    }
+
+    public static void main(String[] args) throws InterruptedException {
+        System.loadLibrary("ExceptionOccurred");
+        try {
+            enable();
+            exceptionExit();
+            disableAndCheck();
+        } catch (RuntimeException e){
+            //expected
+        }
+    }
+}

test/hotspot/jtreg/serviceability/jvmti/events/MethodExit/ExceptionOccurred/libExceptionOccurred.cpp

@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include "jvmti.h"
+#include "jni.h"
+#include "jvmti_common.hpp"
+
+jvmtiEnv* jvmti_env;
+
+bool method_exit_posted = false;
+static void JNICALL
+cbMethodExit(jvmtiEnv* jvmti, JNIEnv* jni, jthread thread, jmethodID method,
+             jboolean was_popped_by_exception, jvalue return_value) {
+  const char * mname = get_method_name(jvmti, jni, method);
+  if (strcmp("upCall", mname) == 0) {
+    if (was_popped_by_exception) {
+      fatal(jni, "The method's was_popped_by_exception value is incorrect.");
+    }
+    jstring upcall_result = (jstring) return_value.l;
+    const char *str = jni->GetStringUTFChars(upcall_result, nullptr);
+    if (str == nullptr) {
+      fatal(jni ,"Failed to convert Java string to C string.");
+    }
+    if (strcmp("MyNewString", str) != 0) {
+      fatal(jni, "The upCall result value is incorrect.");
+    }
+    method_exit_posted = true;
+  }
+  if (strcmp("exceptionExit", mname) != 0) {
+    return;
+  }
+
+  jclass main_class = jni->FindClass("ExceptionOccurred");
+  if (main_class == nullptr) {
+    fatal(jni,"Can't find ExceptionOccurred class.");
+    return;
+  }
+
+  jmethodID upcall_method = jni->GetStaticMethodID(main_class, "upCall", "()Ljava/lang/String;");
+  if (upcall_method == nullptr) {
+    fatal(jni,"Can't find upCall method.");
+  }
+}
+
+JNIEXPORT jint JNICALL
+Agent_OnLoad(JavaVM *vm, char *options, void *reserved) {
+  jvmtiEnv *jvmti = nullptr;
+  jint res = vm->GetEnv((void **) &jvmti, JVMTI_VERSION_21);
+  if (res != JNI_OK) {
+    return JNI_ERR;
+  }
+  jvmtiError err = JVMTI_ERROR_NONE;
+  jvmtiCapabilities capabilities;
+  (void) memset(&capabilities, 0, sizeof (capabilities));
+  capabilities.can_generate_method_exit_events = true;
+  err = jvmti->AddCapabilities(&capabilities);
+  check_jvmti_error(err, "AddCapabilities");
+  jvmtiEventCallbacks callbacks;
+  (void) memset(&callbacks, 0, sizeof (callbacks));
+  callbacks.MethodExit = &cbMethodExit;
+  err = jvmti->SetEventCallbacks(&callbacks, (int) sizeof (jvmtiEventCallbacks));
+  check_jvmti_error(err, "SetEventCallbacks");
+  jvmti_env = jvmti;
+ return JNI_OK;
+}
+
+
+extern "C" {
+JNIEXPORT void JNICALL
+Java_ExceptionOccurred_enable(JNIEnv *jni, jclass clazz) {
+  jthread thread = get_current_thread(jvmti_env, jni);
+  jvmti_env->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_METHOD_EXIT, thread);
+}
+
+
+JNIEXPORT void JNICALL
+Java_ExceptionOccurred_disableAndCheck(JNIEnv *jni, jclass clazz) {
+  jthread thread = get_current_thread(jvmti_env, jni);
+  jvmti_env->SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_METHOD_EXIT, thread);
+  if (!method_exit_posted) {
+    fatal(jni, "Failed to post method exit event.");
+  }
+}
+
+}