8163511: Allocation of compile task fails with assert: "Leaking compilation tasks?"

Use weak handles for compile tasks to allow unloading of referenced methods.

Reviewed-by: kvn, coleenp, eosterlund

src/hotspot/share/ci/ciEnv.cpp

@@ -119,6 +119,9 @@ ciEnv::ciEnv(CompileTask* task, int system_dictionary_modification_counter)
   _system_dictionary_modification_counter = system_dictionary_modification_counter;
   _num_inlined_bytecodes = 0;
   assert(task == NULL || thread->task() == task, "sanity");
+  if (task != NULL) {
+    task->mark_started(os::elapsed_counter());
+  }
   _task = task;
   _log = NULL;
 

src/hotspot/share/compiler/compileBroker.cpp

@@ -258,14 +258,14 @@ CompileTaskWrapper::CompileTaskWrapper(CompileTask* task) {
   }
 #endif
   CompileLog*     log  = thread->log();
-  if (log != NULL)  task->log_task_start(log);
+  if (log != NULL && !task->is_unloaded())  task->log_task_start(log);
 }
 
 CompileTaskWrapper::~CompileTaskWrapper() {
   CompilerThread* thread = CompilerThread::current();
   CompileTask* task = thread->task();
   CompileLog*  log  = thread->log();
-  if (log != NULL)  task->log_task_done(log);
+  if (log != NULL && !task->is_unloaded())  task->log_task_done(log);
   thread->set_task(NULL);
   task->set_code_handle(NULL);
   thread->set_env(NULL);
@@ -444,6 +444,9 @@ CompileTask* CompileQueue::get() {
   {
     NoSafepointVerifier nsv;
     task = CompilationPolicy::policy()->select_task(this);
+    if (task != NULL) {
+      task = task->select_for_compilation();
+    }
   }
 
   if (task != NULL) {
@@ -455,7 +458,6 @@ CompileTask* CompileQueue::get() {
     remove(task);
     purge_stale_tasks(); // may temporarily release MCQ lock
   }
-
   return task;
 }
 
@@ -483,7 +485,7 @@ void CompileQueue::purge_stale_tasks() {
 }
 
 void CompileQueue::remove(CompileTask* task) {
-   assert(MethodCompileQueue_lock->owned_by_self(), "must own lock");
+  assert(MethodCompileQueue_lock->owned_by_self(), "must own lock");
   if (task->prev() != NULL) {
     task->prev()->set_next(task->next());
   } else {
@@ -534,7 +536,7 @@ void CompileBroker::print_compile_queues(outputStream* st) {
 
   char buf[2000];
   int buflen = sizeof(buf);
-  Threads::print_threads_compiling(st, buf, buflen);
+  Threads::print_threads_compiling(st, buf, buflen, /* short_form = */ true);
 
   st->cr();
   if (_c1_compile_queue != NULL) {

src/hotspot/share/compiler/compileTask.cpp

@@ -63,28 +63,31 @@ CompileTask* CompileTask::allocate() {
 /**
 * Add a task to the free list.
 */
-
 void CompileTask::free(CompileTask* task) {
- MutexLocker locker(CompileTaskAlloc_lock);
+  MutexLocker locker(CompileTaskAlloc_lock);
- if (!task->is_free()) {
+  if (!task->is_free()) {
-   task->set_code(NULL);
+    task->set_code(NULL);
-   assert(!task->lock()->is_locked(), "Should not be locked when freed");
+    assert(!task->lock()->is_locked(), "Should not be locked when freed");
-   JNIHandles::destroy_global(task->_method_holder);
+    if ((task->_method_holder != NULL && JNIHandles::is_weak_global_handle(task->_method_holder)) ||
-   JNIHandles::destroy_global(task->_hot_method_holder);
+        (task->_hot_method_holder != NULL && JNIHandles::is_weak_global_handle(task->_hot_method_holder))) {
+      JNIHandles::destroy_weak_global(task->_method_holder);
+      JNIHandles::destroy_weak_global(task->_hot_method_holder);
+    } else {
+      JNIHandles::destroy_global(task->_method_holder);
+      JNIHandles::destroy_global(task->_hot_method_holder);
+    }
+    if (task->_failure_reason_on_C_heap && task->_failure_reason != NULL) {
+      os::free((void*) task->_failure_reason);
+    }
+    task->_failure_reason = NULL;
+    task->_failure_reason_on_C_heap = false;
 
-   if (task->_failure_reason_on_C_heap && task->_failure_reason != NULL) {
+    task->set_is_free(true);
-     os::free((void*) task->_failure_reason);
+    task->set_next(_task_free_list);
-   }
+    _task_free_list = task;
-   task->_failure_reason = NULL;
+  }
-   task->_failure_reason_on_C_heap = false;
-
-   task->set_is_free(true);
-   task->set_next(_task_free_list);
-   _task_free_list = task;
- }
 }
 
-
 void CompileTask::initialize(int compile_id,
                              const methodHandle& method,
                              int osr_bci,
@@ -98,7 +101,7 @@ void CompileTask::initialize(int compile_id,
   Thread* thread = Thread::current();
   _compile_id = compile_id;
   _method = method();
-  _method_holder = JNIHandles::make_global(Handle(thread, method->method_holder()->klass_holder()));
+  _method_holder = JNIHandles::make_weak_global(Handle(thread, method->method_holder()->klass_holder()));
   _osr_bci = osr_bci;
   _is_blocking = is_blocking;
   JVMCI_ONLY(_has_waiter = CompileBroker::compiler(comp_level)->is_jvmci();)
@@ -113,20 +116,20 @@ void CompileTask::initialize(int compile_id,
   _hot_method = NULL;
   _hot_method_holder = NULL;
   _hot_count = hot_count;
-  _time_queued = 0;  // tidy
+  _time_queued = os::elapsed_counter();
+  _time_started = 0;
   _compile_reason = compile_reason;
   _failure_reason = NULL;
   _failure_reason_on_C_heap = false;
 
   if (LogCompilation) {
-    _time_queued = os::elapsed_counter();
     if (hot_method.not_null()) {
       if (hot_method == method) {
         _hot_method = _method;
       } else {
         _hot_method = hot_method();
         // only add loader or mirror if different from _method_holder
-        _hot_method_holder = JNIHandles::make_global(Handle(thread, hot_method->method_holder()->klass_holder()));
+        _hot_method_holder = JNIHandles::make_weak_global(Handle(thread, hot_method->method_holder()->klass_holder()));
       }
     }
   }
@@ -141,6 +144,24 @@ AbstractCompiler* CompileTask::compiler() {
   return CompileBroker::compiler(_comp_level);
 }
 
+// Replace weak handles by strong handles to avoid unloading during compilation.
+CompileTask* CompileTask::select_for_compilation() {
+  if (is_unloaded()) {
+    // Guard against concurrent class unloading
+    return NULL;
+  }
+  Thread* thread = Thread::current();
+  assert(_method->method_holder()->is_loader_alive(), "should be alive");
+  Handle method_holder(thread, _method->method_holder()->klass_holder());
+  JNIHandles::destroy_weak_global(_method_holder);
+  JNIHandles::destroy_weak_global(_hot_method_holder);
+  _method_holder = JNIHandles::make_global(method_holder);
+  if (_hot_method != NULL) {
+    _hot_method_holder = JNIHandles::make_global(Handle(thread, _hot_method->method_holder()->klass_holder()));
+  }
+  return this;
+}
+
 // ------------------------------------------------------------------
 // CompileTask::code/set_code
 //
@@ -162,14 +183,22 @@ void CompileTask::set_code(nmethod* nm) {
 
 void CompileTask::mark_on_stack() {
   // Mark these methods as something redefine classes cannot remove.
+  assert(!is_unloaded(), "unloaded method on the stack");
   _method->set_on_stack(true);
   if (_hot_method != NULL) {
     _hot_method->set_on_stack(true);
   }
 }
 
+bool CompileTask::is_unloaded() const {
+  return _method_holder != NULL && JNIHandles::is_weak_global_handle(_method_holder) && JNIHandles::is_global_weak_cleared(_method_holder);
+}
+
 // RedefineClasses support
 void CompileTask::metadata_do(void f(Metadata*)) {
+  if (is_unloaded()) {
+    return;
+  }
   f(method());
   if (hot_method() != NULL && hot_method() != method()) {
     f(hot_method());
@@ -206,10 +235,20 @@ void CompileTask::print_tty() {
 // ------------------------------------------------------------------
 // CompileTask::print_impl
 void CompileTask::print_impl(outputStream* st, Method* method, int compile_id, int comp_level,
-                                         bool is_osr_method, int osr_bci, bool is_blocking,
+                             bool is_osr_method, int osr_bci, bool is_blocking,
-                                         const char* msg, bool short_form, bool cr) {
+                             const char* msg, bool short_form, bool cr,
+                             jlong time_queued, jlong time_started) {
   if (!short_form) {
-    st->print("%7d ", (int) st->time_stamp().milliseconds());  // print timestamp
+    // Print current time
+    st->print("%7d ", (int)st->time_stamp().milliseconds());
+    if (Verbose && time_queued != 0) {
+      // Print time in queue and time being processed by compiler thread
+      jlong now = os::elapsed_counter();
+      st->print("%d ", (int)TimeHelper::counter_to_millis(now-time_queued));
+      if (time_started != 0) {
+        st->print("%d ", (int)TimeHelper::counter_to_millis(now-time_started));
+      }
+    }
   }
   // print compiler name if requested
   if (CIPrintCompilerName) {
@@ -284,7 +323,7 @@ void CompileTask::print_inline_indent(int inline_level, outputStream* st) {
 // CompileTask::print_compilation
 void CompileTask::print(outputStream* st, const char* msg, bool short_form, bool cr) {
   bool is_osr_method = osr_bci() != InvocationEntryBci;
-  print_impl(st, method(), compile_id(), comp_level(), is_osr_method, osr_bci(), is_blocking(), msg, short_form, cr);
+  print_impl(st, is_unloaded() ? NULL : method(), compile_id(), comp_level(), is_osr_method, osr_bci(), is_blocking(), msg, short_form, cr, _time_queued, _time_started);
 }
 
 // ------------------------------------------------------------------

src/hotspot/share/compiler/compileTask.hpp

@@ -98,7 +98,8 @@ class CompileTask : public CHeapObj<mtCompiler> {
   CompileTask* _next, *_prev;
   bool         _is_free;
   // Fields used for logging why the compilation was initiated:
-  jlong        _time_queued;  // in units of os::elapsed_counter()
+  jlong        _time_queued;  // time when task was enqueued
+  jlong        _time_started; // time when compilation started
   Method*      _hot_method;   // which method actually triggered this task
   jobject      _hot_method_holder;
   int          _hot_count;    // information about its invocation counter
@@ -156,11 +157,13 @@ class CompileTask : public CHeapObj<mtCompiler> {
 
   void         mark_complete()                   { _is_complete = true; }
   void         mark_success()                    { _is_success = true; }
+  void         mark_started(jlong time)          { _time_started = time; }
 
   int          comp_level()                      { return _comp_level;}
   void         set_comp_level(int comp_level)    { _comp_level = comp_level;}
 
   AbstractCompiler* compiler();
+  CompileTask*      select_for_compilation();
 
   int          num_inlined_bytecodes() const     { return _num_inlined_bytecodes; }
   void         set_num_inlined_bytecodes(int n)  { _num_inlined_bytecodes = n; }
@@ -171,6 +174,7 @@ class CompileTask : public CHeapObj<mtCompiler> {
   void         set_prev(CompileTask* prev)       { _prev = prev; }
   bool         is_free() const                   { return _is_free; }
   void         set_is_free(bool val)             { _is_free = val; }
+  bool         is_unloaded() const;
 
   // RedefineClasses support
   void         metadata_do(void f(Metadata*));
@@ -179,7 +183,8 @@ class CompileTask : public CHeapObj<mtCompiler> {
 private:
   static void  print_impl(outputStream* st, Method* method, int compile_id, int comp_level,
                                       bool is_osr_method = false, int osr_bci = -1, bool is_blocking = false,
-                                      const char* msg = NULL, bool short_form = false, bool cr = true);
+                                      const char* msg = NULL, bool short_form = false, bool cr = true,
+                                      jlong time_queued = 0, jlong time_started = 0);
 
 public:
   void         print(outputStream* st = tty, const char* msg = NULL, bool short_form = false, bool cr = true);

src/hotspot/share/runtime/compilationPolicy.cpp

@@ -176,6 +176,14 @@ bool CompilationPolicy::is_compilation_enabled() {
 }
 
 CompileTask* CompilationPolicy::select_task_helper(CompileQueue* compile_queue) {
+  // Remove unloaded methods from the queue
+  for (CompileTask* task = compile_queue->first(); task != NULL; ) {
+    CompileTask* next = task->next();
+    if (task->is_unloaded()) {
+      compile_queue->remove_and_mark_stale(task);
+    }
+    task = next;
+  }
 #if INCLUDE_JVMCI
   if (UseJVMCICompiler && !BackgroundCompilation) {
     /*

src/hotspot/share/runtime/thread.cpp

@@ -4773,7 +4773,7 @@ void Threads::print_on_error(outputStream* st, Thread* current, char* buf,
   print_threads_compiling(st, buf, buflen);
 }
 
-void Threads::print_threads_compiling(outputStream* st, char* buf, int buflen) {
+void Threads::print_threads_compiling(outputStream* st, char* buf, int buflen, bool short_form) {
   ALL_JAVA_THREADS(thread) {
     if (thread->is_Compiler_thread()) {
       CompilerThread* ct = (CompilerThread*) thread;
@@ -4786,7 +4786,7 @@ void Threads::print_threads_compiling(outputStream* st, char* buf, int buflen) {
       if (task != NULL) {
         thread->print_name_on_error(st, buf, buflen);
         st->print("  ");
-        task->print(st, NULL, true, true);
+        task->print(st, NULL, short_form, true);
       }
     }
   }

src/hotspot/share/runtime/thread.hpp

@@ -2298,7 +2298,7 @@ class Threads: AllStatic {
   static void print_on_error(outputStream* st, Thread* current, char* buf, int buflen);
   static void print_on_error(Thread* this_thread, outputStream* st, Thread* current, char* buf,
                              int buflen, bool* found_current);
-  static void print_threads_compiling(outputStream* st, char* buf, int buflen);
+  static void print_threads_compiling(outputStream* st, char* buf, int buflen, bool short_form = false);
 
   // Get Java threads that are waiting to enter a monitor.
   static GrowableArray<JavaThread*>* get_pending_threads(ThreadsList * t_list,

src/hotspot/share/runtime/tieredThresholdPolicy.cpp

@@ -295,28 +295,24 @@ CompileTask* TieredThresholdPolicy::select_task(CompileQueue* compile_queue) {
   for (CompileTask* task = compile_queue->first(); task != NULL;) {
     CompileTask* next_task = task->next();
     Method* method = task->method();
+    // If a method was unloaded or has been stale for some time, remove it from the queue.
+    // Blocking tasks and tasks submitted from whitebox API don't become stale
+    if (task->is_unloaded() || (task->can_become_stale() && is_stale(t, TieredCompileTaskTimeout, method) && !is_old(method))) {
+      if (!task->is_unloaded()) {
+        if (PrintTieredEvents) {
+          print_event(REMOVE_FROM_QUEUE, method, method, task->osr_bci(), (CompLevel) task->comp_level());
+        }
+        method->clear_queued_for_compilation();
+      }
+      compile_queue->remove_and_mark_stale(task);
+      task = next_task;
+      continue;
+    }
     update_rate(t, method);
-    if (max_task == NULL) {
+    if (max_task == NULL || compare_methods(method, max_method)) {
+      // Select a method with the highest rate
       max_task = task;
       max_method = method;
-    } else {
-      // If a method has been stale for some time, remove it from the queue.
-      // Blocking tasks and tasks submitted from whitebox API don't become stale
-      if (task->can_become_stale() && is_stale(t, TieredCompileTaskTimeout, method) && !is_old(method)) {
-        if (PrintTieredEvents) {
-          print_event(REMOVE_FROM_QUEUE, method, method, task->osr_bci(), (CompLevel)task->comp_level());
-        }
-        compile_queue->remove_and_mark_stale(task);
-        method->clear_queued_for_compilation();
-        task = next_task;
-        continue;
-      }
-
-      // Select a method with a higher rate
-      if (compare_methods(method, max_method)) {
-        max_task = task;
-        max_method = method;
-      }
     }
 
     if (task->is_blocking()) {
@@ -501,7 +497,7 @@ void TieredThresholdPolicy::update_rate(jlong t, Method* m) {
   }
 }
 
-// Check if this method has been stale from a given number of milliseconds.
+// Check if this method has been stale for a given number of milliseconds.
 // See select_task().
 bool TieredThresholdPolicy::is_stale(jlong t, jlong timeout, Method* m) {
   jlong delta_s = t - SafepointTracing::end_of_last_safepoint_epoch_ms();

test/hotspot/jtreg/compiler/classUnloading/methodUnloading/TestOverloadCompileQueues.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 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
+ * 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 TestOverloadCompileQueues
+ * @bug 8163511
+ * @summary Test overloading the C1 and C2 compile queues with tasks.
+ * @run main/othervm -XX:-TieredCompilation -XX:CompileThreshold=2 -XX:CICompilerCount=1
+ *                   compiler.classUnloading.methodUnloading.TestOverloadCompileQueues
+ * @run main/othervm -XX:TieredCompileTaskTimeout=1000 -XX:CompileThresholdScaling=0.001 -XX:CICompilerCount=2
+ *                   compiler.classUnloading.methodUnloading.TestOverloadCompileQueues
+ */
+
+package compiler.classUnloading.methodUnloading;
+
+import java.lang.reflect.Method;
+import java.net.URL;
+import java.net.URLClassLoader;
+
+public class TestOverloadCompileQueues {
+    public static final int ITERS = 500; // Increase for longer stress testing
+
+    // Some methods to fill up the compile queue
+    public static void test0() { }
+    public static void test1() { }
+    public static void test2() { }
+    public static void test3() { }
+    public static void test4() { }
+    public static void test5() { }
+    public static void test6() { }
+    public static void test7() { }
+    public static void test8() { }
+    public static void test9() { }
+    public static void test10() { }
+    public static void test11() { }
+    public static void test12() { }
+    public static void test13() { }
+    public static void test14() { }
+    public static void test15() { }
+    public static void test16() { }
+    public static void test17() { }
+    public static void test18() { }
+    public static void test19() { }
+
+    public static void main(String[] args) throws Throwable {
+        Class<?> thisClass = TestOverloadCompileQueues.class;
+        ClassLoader defaultLoader = thisClass.getClassLoader();
+        URL classesDir = thisClass.getProtectionDomain().getCodeSource().getLocation();
+
+        for (int i = 0; i < ITERS; ++i) {
+            // Load test class with own class loader
+            URLClassLoader myLoader = URLClassLoader.newInstance(new URL[] {classesDir}, defaultLoader.getParent());
+            Class<?> testClass = Class.forName(thisClass.getCanonicalName(), true, myLoader);
+
+            // Execute all test methods to trigger compilation and fill up compile queue
+            for (int j = 1; j < 20; ++j) {
+                Method method = testClass.getDeclaredMethod("test" + j);
+                method.invoke(null);
+                method.invoke(null);
+            }
+
+            // Unload dead classes from ealier iterations
+            System.gc();
+        }
+    }
+}