8173658: JvmtiExport::post_class_unload() is broken for non-JavaThrea…

…d initiators

Call extension ClassUnload event as a deferred event from the ServiceThread and remove unsafe arguments

Backport-of: d1d57d9

src/hotspot/share/prims/jvmtiExport.cpp

@@ -1349,54 +1349,42 @@ void JvmtiExport::post_class_unload(Klass* klass) {
   if (JvmtiEnv::get_phase() < JVMTI_PHASE_PRIMORDIAL) {
     return;
   }
-  Thread *thread = Thread::current();
+
+  // postings to the service thread so that it can perform them in a safe
+  // context and in-order.
+  MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
+  ResourceMark rm;
+  // JvmtiDeferredEvent copies the string.
+  JvmtiDeferredEvent event = JvmtiDeferredEvent::class_unload_event(klass->name()->as_C_string());
+  JvmtiDeferredEventQueue::enqueue(event);
+}
+
+
+void JvmtiExport::post_class_unload_internal(const char* name) {
+  if (JvmtiEnv::get_phase() < JVMTI_PHASE_PRIMORDIAL) {
+    return;
+  }
+  assert(Thread::current()->is_service_thread(), "must be called from ServiceThread");
+  JavaThread *thread = JavaThread::current();
   HandleMark hm(thread);
 
   EVT_TRIG_TRACE(EXT_EVENT_CLASS_UNLOAD, ("[?] Trg Class Unload triggered" ));
   if (JvmtiEventController::is_enabled((jvmtiEvent)EXT_EVENT_CLASS_UNLOAD)) {
-    assert(thread->is_VM_thread(), "wrong thread");
-
-    // get JavaThread for whom we are proxy
-    Thread *calling_thread = ((VMThread *)thread)->vm_operation()->calling_thread();
-    if (!calling_thread->is_Java_thread()) {
-      // cannot post an event to a non-JavaThread
-      return;
-    }
-    JavaThread *real_thread = (JavaThread *)calling_thread;
 
     JvmtiEnvIterator it;
     for (JvmtiEnv* env = it.first(); env != NULL; env = it.next(env)) {
       if (env->phase() == JVMTI_PHASE_PRIMORDIAL) {
         continue;
       }
       if (env->is_enabled((jvmtiEvent)EXT_EVENT_CLASS_UNLOAD)) {
-        EVT_TRACE(EXT_EVENT_CLASS_UNLOAD, ("[?] Evt Class Unload sent %s",
-                  klass==NULL? "NULL" : klass->external_name() ));
-
-        // do everything manually, since this is a proxy - needs special care
-        JNIEnv* jni_env = real_thread->jni_environment();
-        jthread jt = (jthread)JNIHandles::make_local(real_thread, real_thread->threadObj());
-        jclass jk = (jclass)JNIHandles::make_local(real_thread, klass->java_mirror());
-
-        // Before we call the JVMTI agent, we have to set the state in the
-        // thread for which we are proxying.
-        JavaThreadState prev_state = real_thread->thread_state();
-        assert(((Thread *)real_thread)->is_ConcurrentGC_thread() ||
-               (real_thread->is_Java_thread() && prev_state == _thread_blocked),
-               "should be ConcurrentGCThread or JavaThread at safepoint");
-        real_thread->set_thread_state(_thread_in_native);
+        EVT_TRACE(EXT_EVENT_CLASS_UNLOAD, ("[?] Evt Class Unload sent %s", name));
 
+        JvmtiEventMark jem(thread);
+        JvmtiJavaThreadEventTransition jet(thread);
         jvmtiExtensionEvent callback = env->ext_callbacks()->ClassUnload;
         if (callback != NULL) {
-          (*callback)(env->jvmti_external(), jni_env, jt, jk);
+          (*callback)(env->jvmti_external(), jem.jni_env(), name);
         }
-
-        assert(real_thread->thread_state() == _thread_in_native,
-               "JavaThread should be in native");
-        real_thread->set_thread_state(prev_state);
-
-        JNIHandles::destroy_local(jk);
-        JNIHandles::destroy_local(jt);
       }
     }
   }

src/hotspot/share/prims/jvmtiExport.hpp

@@ -160,6 +160,7 @@ class JvmtiExport : public AllStatic {
   // internal implementation.  Also called from JvmtiDeferredEvent::post()
   static void post_dynamic_code_generated_internal(const char *name, const void *code_begin, const void *code_end) NOT_JVMTI_RETURN;
 
+  static void post_class_unload_internal(const char *name) NOT_JVMTI_RETURN;
  private:
 
   // GenerateEvents support to allow posting of CompiledMethodLoad and

src/hotspot/share/prims/jvmtiExtensions.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2019, 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
@@ -70,9 +70,8 @@ void JvmtiExtensions::register_extensions() {
   // register our extension event
 
   static jvmtiParamInfo event_params[] = {
-    { (char*)"JNI Environment", JVMTI_KIND_IN, JVMTI_TYPE_JNIENV, JNI_FALSE },
-    { (char*)"Thread", JVMTI_KIND_IN, JVMTI_TYPE_JTHREAD, JNI_FALSE },
-    { (char*)"Class", JVMTI_KIND_IN, JVMTI_TYPE_JCLASS, JNI_FALSE }
+    { (char*)"JNI Environment", JVMTI_KIND_IN_PTR, JVMTI_TYPE_JNIENV, JNI_FALSE },
+    { (char*)"Class", JVMTI_KIND_IN_PTR, JVMTI_TYPE_CCHAR, JNI_FALSE }
   };
   static jvmtiExtensionEventInfo ext_event = {
     EXT_EVENT_CLASS_UNLOAD,

src/hotspot/share/prims/jvmtiImpl.cpp

@@ -951,6 +951,16 @@ JvmtiDeferredEvent JvmtiDeferredEvent::dynamic_code_generated_event(
   return event;
 }
 
+JvmtiDeferredEvent JvmtiDeferredEvent::class_unload_event(const char* name) {
+  JvmtiDeferredEvent event = JvmtiDeferredEvent(TYPE_CLASS_UNLOAD);
+  // Need to make a copy of the name since we don't know how long
+  // the event poster will keep it around after we enqueue the
+  // deferred event and return. strdup() failure is handled in
+  // the post() routine below.
+  event._event_data.class_unload.name = os::strdup(name);
+  return event;
+}
+
 void JvmtiDeferredEvent::post() {
   assert(Thread::current()->is_service_thread(),
          "Service thread must post enqueued events");
@@ -982,6 +992,17 @@ void JvmtiDeferredEvent::post() {
       }
       break;
     }
+    case TYPE_CLASS_UNLOAD: {
+      JvmtiExport::post_class_unload_internal(
+        // if strdup failed give the event a default name
+        (_event_data.class_unload.name == NULL)
+          ? "unknown_class" : _event_data.class_unload.name);
+      if (_event_data.class_unload.name != NULL) {
+        // release our copy
+        os::free((void *)_event_data.class_unload.name);
+      }
+      break;
+    }
     default:
       ShouldNotReachHere();
   }

src/hotspot/share/prims/jvmtiImpl.hpp

@@ -445,7 +445,8 @@ class JvmtiDeferredEvent {
     TYPE_NONE,
     TYPE_COMPILED_METHOD_LOAD,
     TYPE_COMPILED_METHOD_UNLOAD,
-    TYPE_DYNAMIC_CODE_GENERATED
+    TYPE_DYNAMIC_CODE_GENERATED,
+    TYPE_CLASS_UNLOAD
   } Type;
 
   Type _type;
@@ -461,6 +462,9 @@ class JvmtiDeferredEvent {
       const void* code_begin;
       const void* code_end;
     } dynamic_code_generated;
+    struct {
+      const char* name;
+    } class_unload;
   } _event_data;
 
   JvmtiDeferredEvent(Type t) : _type(t) {}
@@ -477,6 +481,8 @@ class JvmtiDeferredEvent {
   static JvmtiDeferredEvent dynamic_code_generated_event(
       const char* name, const void* begin, const void* end)
           NOT_JVMTI_RETURN_(JvmtiDeferredEvent());
+  static JvmtiDeferredEvent class_unload_event(
+      const char* name) NOT_JVMTI_RETURN_(JvmtiDeferredEvent());
 
   // Actually posts the event.
   void post() NOT_JVMTI_RETURN;

src/hotspot/share/runtime/thread.cpp

@@ -1732,7 +1732,7 @@ void JavaThread::initialize() {
   clear_must_deopt_id();
   set_monitor_chunks(NULL);
   _on_thread_list = false;
-  set_thread_state(_thread_new);
+  _thread_state = _thread_new;
   _terminated = _not_terminated;
   _array_for_gc = NULL;
   _suspend_equivalent = false;

src/hotspot/share/runtime/thread.inline.hpp

@@ -125,6 +125,8 @@ inline JavaThreadState JavaThread::thread_state() const    {
 }
 
 inline void JavaThread::set_thread_state(JavaThreadState s) {
+  assert(current_or_null() == NULL || current_or_null() == this,
+         "state change should only be called by the current thread");
 #if defined(PPC64) || defined (AARCH64)
   // Use membars when accessing volatile _thread_state. See
   // Threads::create_vm() for size checks.

test/hotspot/jtreg/ProblemList.txt

@@ -174,7 +174,6 @@ vmTestbase/metaspace/gc/firstGC_default/TestDescription.java 8208250 generic-all
 
 vmTestbase/nsk/jvmti/ResourceExhausted/resexhausted003/TestDescription.java 6606767 generic-all
 vmTestbase/nsk/jvmti/ResourceExhausted/resexhausted004/TestDescription.java 6606767 generic-all
-vmTestbase/nsk/jvmti/scenarios/extension/EX03/ex03t001/TestDescription.java 8173658 generic-all
 vmTestbase/nsk/jvmti/AttachOnDemand/attach045/TestDescription.java 8202971 generic-all
 vmTestbase/nsk/jvmti/scenarios/jni_interception/JI05/ji05t001/TestDescription.java 8219652 aix-ppc64
 vmTestbase/nsk/jvmti/scenarios/jni_interception/JI06/ji06t001/TestDescription.java 8219652 aix-ppc64

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/extension/EX03/ex03t001/ex03t001.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2019, 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
@@ -41,18 +41,15 @@ static jrawMonitorID eventMon;
 /* ============================================================================= */
 
 static void JNICALL
-ClassUnload(jvmtiEnv* jvmti_env, JNIEnv *jni_env, jthread thread, jclass klass, ...) {
-    /*
-     * With the CMS GC the event can be posted on
-     * a ConcurrentGC thread that is not a JavaThread.
-     * In this case the thread argument can be NULL, so that,
-     * we should not expect the thread argument to be non-NULL.
-     */
-    if (klass == NULL) {
+ClassUnload(jvmtiEnv* jvmti_env, JNIEnv* jni_env, const char* name, ...) {
+    // The name argument should never be null
+    if (name == NULL) {
         nsk_jvmti_setFailStatus();
-        NSK_COMPLAIN0("ClassUnload: 'klass' input parameter is NULL.\n");
-
+        NSK_COMPLAIN0("ClassUnload: 'name' input parameter is NULL.\n");
+    } else {
+        NSK_DISPLAY1("Class unloaded %s\n", name);
     }
+
     NSK_DISPLAY0("Received ClassUnload event.\n");
     if (eventEnabled == JNI_TRUE) {
         eventReceived1 = JNI_TRUE;
@@ -107,6 +104,20 @@ jboolean isClassUnloadingEnabled() {
     return enabled;
 }
 
+jboolean checkParams(jvmtiExtensionEventInfo event) {
+    // Check parameters are:
+    // JNIEnv *jni_env, const char* name
+    if (event.param_count != 2 ||
+          event.params[0].kind != JVMTI_KIND_IN_PTR ||
+          event.params[0].base_type != JVMTI_TYPE_JNIENV ||
+          event.params[1].kind != JVMTI_KIND_IN_PTR ||
+          event.params[1].base_type != JVMTI_TYPE_CCHAR) {
+        return JNI_FALSE;
+    } else {
+        return JNI_TRUE;
+    }
+}
+
 jboolean enableClassUnloadEvent (jboolean enable) {
     jint extCount, i;
     jvmtiExtensionEventInfo* extList;
@@ -122,6 +133,14 @@ jboolean enableClassUnloadEvent (jboolean enable) {
         if (strcmp(extList[i].id, (char*)"com.sun.hotspot.events.ClassUnload") == 0) {
             found = JNI_TRUE;
 
+            NSK_DISPLAY1("%s", extList[i].short_description);
+
+            if (!checkParams(extList[i])) {
+                NSK_COMPLAIN0("ClassUnload event has wrong parameters.");
+                nsk_jvmti_setFailStatus();
+                return JNI_FALSE;
+            }
+
             if (!NSK_JVMTI_VERIFY(
                     jvmti->SetExtensionEventCallback(extList[i].extension_event_index,
                                                      enable ? (jvmtiExtensionEvent)ClassUnload : NULL))) {