8276658: Clean up JNI local handles code

Reviewed-by: dholmes, pchilanomate

Author: coleenp

src/hotspot/share/ci/ciInstanceKlass.cpp

@@ -87,7 +87,7 @@ ciInstanceKlass::ciInstanceKlass(Klass* k) :
     (void)CURRENT_ENV->get_object(holder);
   }
 
-  Thread *thread = Thread::current();
+  JavaThread *thread = JavaThread::current();
   if (ciObjectFactory::is_initialized()) {
     _loader = JNIHandles::make_local(thread, ik->class_loader());
     _protection_domain = JNIHandles::make_local(thread,

src/hotspot/share/compiler/compileBroker.cpp

@@ -2200,7 +2200,7 @@ void CompileBroker::invoke_compiler_on_method(CompileTask* task) {
   }
 
   // Allocate a new set of JNI handles.
-  push_jni_handle_block();
+  JNIHandleMark jhm(thread);
   Method* target_handle = task->method();
   int compilable = ciEnv::MethodCompilable;
   const char* failure_reason = NULL;
@@ -2319,9 +2319,6 @@ void CompileBroker::invoke_compiler_on_method(CompileTask* task) {
       post_compilation_event(event, task);
     }
   }
-  // Remove the JNI handle block after the ciEnv destructor has run in
-  // the previous block.
-  pop_jni_handle_block();
 
   if (failure_reason != NULL) {
     task->set_failure_reason(failure_reason, failure_reason_on_C_heap);
@@ -2482,38 +2479,6 @@ void CompileBroker::update_compile_perf_data(CompilerThread* thread, const metho
   counters->set_compile_type((jlong) last_compile_type);
 }
 
-// ------------------------------------------------------------------
-// CompileBroker::push_jni_handle_block
-//
-// Push on a new block of JNI handles.
-void CompileBroker::push_jni_handle_block() {
-  JavaThread* thread = JavaThread::current();
-
-  // Allocate a new block for JNI handles.
-  // Inlined code from jni_PushLocalFrame()
-  JNIHandleBlock* java_handles = thread->active_handles();
-  JNIHandleBlock* compile_handles = JNIHandleBlock::allocate_block(thread);
-  assert(compile_handles != NULL && java_handles != NULL, "should not be NULL");
-  compile_handles->set_pop_frame_link(java_handles);  // make sure java handles get gc'd.
-  thread->set_active_handles(compile_handles);
-}
-
-
-// ------------------------------------------------------------------
-// CompileBroker::pop_jni_handle_block
-//
-// Pop off the current block of JNI handles.
-void CompileBroker::pop_jni_handle_block() {
-  JavaThread* thread = JavaThread::current();
-
-  // Release our JNI handle block
-  JNIHandleBlock* compile_handles = thread->active_handles();
-  JNIHandleBlock* java_handles = compile_handles->pop_frame_link();
-  thread->set_active_handles(java_handles);
-  compile_handles->set_pop_frame_link(NULL);
-  JNIHandleBlock::release_block(compile_handles, thread); // may block
-}
-
 // ------------------------------------------------------------------
 // CompileBroker::collect_statistics
 //

src/hotspot/share/compiler/compileBroker.hpp

@@ -259,8 +259,6 @@ class CompileBroker: AllStatic {
                            int compilable, const char* failure_reason);
   static void update_compile_perf_data(CompilerThread *thread, const methodHandle& method, bool is_osr);
 
-  static void push_jni_handle_block();
-  static void pop_jni_handle_block();
   static void collect_statistics(CompilerThread* thread, elapsedTimer time, CompileTask* task);
 
   static void compile_method_base(const methodHandle& method,

src/hotspot/share/gc/shared/concurrentGCThread.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2021, 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
@@ -42,9 +42,6 @@ void ConcurrentGCThread::create_and_start(ThreadPriority prio) {
 }
 
 void ConcurrentGCThread::run() {
-  // Setup handle area
-  set_active_handles(JNIHandleBlock::allocate_block());
-
   // Wait for initialization to complete
   wait_init_completed();
 

src/hotspot/share/jfr/dcmd/jfrDcmds.cpp

@@ -55,53 +55,6 @@ bool register_jfr_dcmds() {
   return true;
 }
 
-// JNIHandle management
-
-// ------------------------------------------------------------------
-// push_jni_handle_block
-//
-// Push on a new block of JNI handles.
-static void push_jni_handle_block(JavaThread* const thread) {
-  DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(thread));
-
-  // Allocate a new block for JNI handles.
-  // Inlined code from jni_PushLocalFrame()
-  JNIHandleBlock* prev_handles = thread->active_handles();
-  JNIHandleBlock* entry_handles = JNIHandleBlock::allocate_block(thread);
-  assert(entry_handles != NULL && prev_handles != NULL, "should not be NULL");
-  entry_handles->set_pop_frame_link(prev_handles);  // make sure prev handles get gc'd.
-  thread->set_active_handles(entry_handles);
-}
-
-// ------------------------------------------------------------------
-// pop_jni_handle_block
-//
-// Pop off the current block of JNI handles.
-static void pop_jni_handle_block(JavaThread* const thread) {
-  DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(thread));
-
-  // Release our JNI handle block
-  JNIHandleBlock* entry_handles = thread->active_handles();
-  JNIHandleBlock* prev_handles = entry_handles->pop_frame_link();
-  // restore
-  thread->set_active_handles(prev_handles);
-  entry_handles->set_pop_frame_link(NULL);
-  JNIHandleBlock::release_block(entry_handles, thread); // may block
-}
-
-class JNIHandleBlockManager : public StackObj {
- private:
-  JavaThread* const _thread;
- public:
-  JNIHandleBlockManager(JavaThread* thread) : _thread(thread) {
-    push_jni_handle_block(_thread);
-  }
-
-  ~JNIHandleBlockManager() {
-    pop_jni_handle_block(_thread);
-  }
-};
-
 static bool is_module_available(outputStream* output, TRAPS) {
   return JfrJavaSupport::is_jdk_jfr_module_available(output, THREAD);
 }
@@ -223,7 +176,7 @@ void JfrDCmd::invoke(JfrJavaArguments& method, TRAPS) const {
   constructor_args.set_klass(javaClass(), CHECK);
 
   HandleMark hm(THREAD);
-  JNIHandleBlockManager jni_handle_management(THREAD);
+  JNIHandleMark jni_handle_management(THREAD);
 
   const oop dcmd = construct_dcmd_instance(&constructor_args, CHECK);
 
@@ -494,7 +447,7 @@ void JfrConfigureFlightRecorderDCmd::execute(DCmdSource source, TRAPS) {
   }
 
   HandleMark hm(THREAD);
-  JNIHandleBlockManager jni_handle_management(THREAD);
+  JNIHandleMark jni_handle_management(THREAD);
 
   JavaValue result(T_OBJECT);
   JfrJavaArguments constructor_args(&result);

src/hotspot/share/jfr/jni/jfrJavaSupport.cpp

@@ -71,7 +71,7 @@ static void check_new_unstarted_java_thread(JavaThread* t) {
  */
 jobject JfrJavaSupport::local_jni_handle(const oop obj, JavaThread* t) {
   DEBUG_ONLY(check_java_thread_in_vm(t));
-  return t->active_handles()->allocate_handle(obj);
+  return t->active_handles()->allocate_handle(t, obj);
 }
 
 jobject JfrJavaSupport::local_jni_handle(const jobject handle, JavaThread* t) {

src/hotspot/share/jvmci/jvmciCodeInstaller.cpp

@@ -899,7 +899,7 @@ GrowableArray<ScopeValue*>* CodeInstaller::record_virtual_objects(JVMCIObject de
     }
     Klass* klass = jvmci_env()->asKlass(type);
     oop javaMirror = klass->java_mirror();
-    ScopeValue *klass_sv = new ConstantOopWriteValue(JNIHandles::make_local(Thread::current(), javaMirror));
+    ScopeValue *klass_sv = new ConstantOopWriteValue(JNIHandles::make_local(javaMirror));
     ObjectValue* sv = is_auto_box ? new AutoBoxObjectValue(id, klass_sv) : new ObjectValue(id, klass_sv);
     if (id < 0 || id >= objects->length()) {
       JVMCI_ERROR_NULL("virtual object id %d out of bounds", id);

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -86,29 +86,6 @@ static void requireInHotSpot(const char* caller, JVMCI_TRAPS) {
   }
 }
 
-void JNIHandleMark::push_jni_handle_block(JavaThread* thread) {
-  if (thread != NULL) {
-    // Allocate a new block for JNI handles.
-    // Inlined code from jni_PushLocalFrame()
-    JNIHandleBlock* java_handles = thread->active_handles();
-    JNIHandleBlock* compile_handles = JNIHandleBlock::allocate_block(thread);
-    assert(compile_handles != NULL && java_handles != NULL, "should not be NULL");
-    compile_handles->set_pop_frame_link(java_handles);
-    thread->set_active_handles(compile_handles);
-  }
-}
-
-void JNIHandleMark::pop_jni_handle_block(JavaThread* thread) {
-  if (thread != NULL) {
-    // Release our JNI handle block
-    JNIHandleBlock* compile_handles = thread->active_handles();
-    JNIHandleBlock* java_handles = compile_handles->pop_frame_link();
-    thread->set_active_handles(java_handles);
-    compile_handles->set_pop_frame_link(NULL);
-    JNIHandleBlock::release_block(compile_handles, thread); // may block
-  }
-}
-
 class JVMCITraceMark : public StackObj {
   const char* _msg;
  public:

src/hotspot/share/jvmci/jvmciCompilerToVM.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2021, 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
@@ -173,15 +173,4 @@ class JavaArgumentUnboxer : public SignatureIterator {
   }
 };
 
-class JNIHandleMark : public StackObj {
-  JavaThread* _thread;
-  public:
-    JNIHandleMark(JavaThread* thread) : _thread(thread) { push_jni_handle_block(thread); }
-    ~JNIHandleMark() { pop_jni_handle_block(_thread); }
-
-  private:
-    static void push_jni_handle_block(JavaThread* thread);
-    static void pop_jni_handle_block(JavaThread* thread);
-};
-
 #endif // SHARE_JVMCI_JVMCICOMPILERTOVM_HPP

src/hotspot/share/prims/jni.cpp

@@ -639,11 +639,8 @@ JNI_ENTRY(jint, jni_PushLocalFrame(JNIEnv *env, jint capacity))
     HOTSPOT_JNI_PUSHLOCALFRAME_RETURN((uint32_t)JNI_ERR);
     return JNI_ERR;
   }
-  JNIHandleBlock* old_handles = thread->active_handles();
-  JNIHandleBlock* new_handles = JNIHandleBlock::allocate_block(thread);
-  assert(new_handles != NULL, "should not be NULL");
-  new_handles->set_pop_frame_link(old_handles);
-  thread->set_active_handles(new_handles);
+
+  thread->push_jni_handle_block();
   jint ret = JNI_OK;
   HOTSPOT_JNI_PUSHLOCALFRAME_RETURN(ret);
   return ret;

src/hotspot/share/prims/jvmtiEnvBase.cpp

@@ -1510,7 +1510,7 @@ JvmtiModuleClosure::get_all_modules(JvmtiEnv* env, jint* module_count_ptr, jobje
     return JVMTI_ERROR_OUT_OF_MEMORY;
   }
   for (jint idx = 0; idx < len; idx++) {
-    array[idx] = JNIHandles::make_local(Thread::current(), _tbl->at(idx).resolve());
+    array[idx] = JNIHandles::make_local(_tbl->at(idx).resolve());
   }
   _tbl = NULL;
   *modules_ptr = array;

src/hotspot/share/prims/jvmtiExport.cpp

@@ -141,52 +141,25 @@ class JvmtiEventMark : public StackObj {
   JavaThread *_thread;
   JNIEnv* _jni_env;
   JvmtiThreadState::ExceptionState _saved_exception_state;
-#if 0
-  JNIHandleBlock* _hblock;
-#endif
 
 public:
   JvmtiEventMark(JavaThread *thread) :  _thread(thread),
                                         _jni_env(thread->jni_environment()),
                                         _saved_exception_state(JvmtiThreadState::ES_CLEARED) {
-#if 0
-    _hblock = thread->active_handles();
-    _hblock->clear_thoroughly(); // so we can be safe
-#else
-    // we want to use the code above - but that needs the JNIHandle changes - later...
-    // for now, steal JNI push local frame code
     JvmtiThreadState *state = thread->jvmti_thread_state();
     // we are before an event.
     // Save current jvmti thread exception state.
     if (state != NULL) {
       _saved_exception_state = state->get_exception_state();
     }
 
-    JNIHandleBlock* old_handles = thread->active_handles();
-    JNIHandleBlock* new_handles = JNIHandleBlock::allocate_block(thread);
-    assert(new_handles != NULL, "should not be NULL");
-    new_handles->set_pop_frame_link(old_handles);
-    thread->set_active_handles(new_handles);
-#endif
+    thread->push_jni_handle_block();
     assert(thread == JavaThread::current(), "thread must be current!");
     thread->frame_anchor()->make_walkable(thread);
   };
 
   ~JvmtiEventMark() {
-#if 0
-    _hblock->clear(); // for consistency with future correct behavior
-#else
-    // we want to use the code above - but that needs the JNIHandle changes - later...
-    // for now, steal JNI pop local frame code
-    JNIHandleBlock* old_handles = _thread->active_handles();
-    JNIHandleBlock* new_handles = old_handles->pop_frame_link();
-    assert(new_handles != NULL, "should not be NULL");
-    _thread->set_active_handles(new_handles);
-    // Note that we set the pop_frame_link to NULL explicitly, otherwise
-    // the release_block call will release the blocks.
-    old_handles->set_pop_frame_link(NULL);
-    JNIHandleBlock::release_block(old_handles, _thread); // may block
-#endif
+    _thread->pop_jni_handle_block();
 
     JvmtiThreadState* state = _thread->jvmti_thread_state();
     // we are continuing after an event.
@@ -196,13 +169,7 @@ class JvmtiEventMark : public StackObj {
     }
   }
 
-#if 0
-  jobject to_jobject(oop obj) { return obj == NULL? NULL : _hblock->allocate_handle_fast(obj); }
-#else
-  // we want to use the code above - but that needs the JNIHandle changes - later...
-  // for now, use regular make_local
   jobject to_jobject(oop obj) { return JNIHandles::make_local(_thread,obj); }
-#endif
 
   jclass to_jclass(Klass* klass) { return (klass == NULL ? NULL : (jclass)to_jobject(klass->java_mirror())); }