8216136: Take Compile_lock for SystemDictionary::_modification_counter with safepoint check

Transition compiler thread from native to VM state to check the Compile_lock with safepoint always.

Reviewed-by: dlong, eosterlund, dholmes

src/hotspot/share/ci/ciEnv.cpp

@@ -915,10 +915,17 @@ bool ciEnv::is_in_vm() {
   return JavaThread::current()->thread_state() == _thread_in_vm;
 }
 
-bool ciEnv::system_dictionary_modification_counter_changed() {
+bool ciEnv::system_dictionary_modification_counter_changed_locked() {
+  assert_locked_or_safepoint(Compile_lock);
   return _system_dictionary_modification_counter != SystemDictionary::number_of_modifications();
 }
 
+bool ciEnv::system_dictionary_modification_counter_changed() {
+  VM_ENTRY_MARK;
+  MutexLocker ml(Compile_lock, THREAD); // lock with safepoint check
+  return system_dictionary_modification_counter_changed_locked();
+}
+
 // ------------------------------------------------------------------
 // ciEnv::validate_compile_task_dependencies
 //
@@ -927,7 +934,7 @@ bool ciEnv::system_dictionary_modification_counter_changed() {
 void ciEnv::validate_compile_task_dependencies(ciMethod* target) {
   if (failing())  return;  // no need for further checks
 
-  bool counter_changed = system_dictionary_modification_counter_changed();
+  bool counter_changed = system_dictionary_modification_counter_changed_locked();
   Dependencies::DepType result = dependencies()->validate_dependencies(_task, counter_changed);
   if (result != Dependencies::end_marker) {
     if (result == Dependencies::call_site_target_value) {

src/hotspot/share/ci/ciEnv.hpp

@@ -292,6 +292,8 @@ private:
   // respect to method dependencies (e.g. concurrent class loading).
   void validate_compile_task_dependencies(ciMethod* target);
 
+  // Call internally when Compile_lock is already held.
+  bool system_dictionary_modification_counter_changed_locked();
 public:
   enum {
     MethodCompilable,

src/hotspot/share/opto/callGenerator.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 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
@@ -90,7 +90,6 @@ JVMState* ParseGenerator::generate(JVMState* jvms) {
   // Grab signature for matching/allocation
 #ifdef ASSERT
   if (parser.tf() != (parser.depth() == 1 ? C->tf() : tf())) {
-    MutexLockerEx ml(Compile_lock, Mutex::_no_safepoint_check_flag);
     assert(C->env()->system_dictionary_modification_counter_changed(),
            "Must invalidate if TypeFuncs differ");
   }

src/hotspot/share/opto/parse1.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -524,7 +524,6 @@ Parse::Parse(JVMState* caller, ciMethod* parse_method, float expected_uses)
   if (depth() == 1) {
     assert(C->is_osr_compilation() == this->is_osr_parse(), "OSR in sync");
     if (C->tf() != tf()) {
-      MutexLockerEx ml(Compile_lock, Mutex::_no_safepoint_check_flag);
       assert(C->env()->system_dictionary_modification_counter_changed(),
              "Must invalidate if TypeFuncs differ");
     }
@@ -1041,7 +1040,6 @@ void Parse::do_exits() {
       // not compilable. Just using an assertion instead would be dangerous
       // as this could lead to an infinite compile loop in non-debug builds.
       {
-        MutexLockerEx ml(Compile_lock, Mutex::_no_safepoint_check_flag);
         if (C->env()->system_dictionary_modification_counter_changed()) {
           C->record_failure(C2Compiler::retry_class_loading_during_parsing());
         } else {

src/hotspot/share/runtime/mutexLocker.cpp

@@ -306,7 +306,7 @@ void mutex_init() {
   def(JvmtiThreadState_lock        , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always);     // Used by JvmtiThreadState/JvmtiEventController
   def(Management_lock              , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always);     // used for JVM management
 
-  def(Compile_lock                 , PaddedMutex  , nonleaf+3,   true,  Monitor::_safepoint_check_sometimes);
+  def(Compile_lock                 , PaddedMutex  , nonleaf+3,   true,  Monitor::_safepoint_check_always);
   def(MethodData_lock              , PaddedMutex  , nonleaf+3,   false, Monitor::_safepoint_check_always);
   def(TouchedMethodLog_lock        , PaddedMutex  , nonleaf+3,   false, Monitor::_safepoint_check_always);