8214556: Crash in DependencyContext::remove_dependent_nmethod still happens
authoreosterlund
Tue, 04 Dec 2018 17:14:11 +0100
changeset 52819 022420a4cc63
parent 52818 cfbe4d8ffd1d
child 52820 8527b6100a59
8214556: Crash in DependencyContext::remove_dependent_nmethod still happens Reviewed-by: kvn, kbarrett
src/hotspot/share/code/dependencyContext.cpp
src/hotspot/share/code/dependencyContext.hpp
src/hotspot/share/prims/methodHandles.cpp
--- a/src/hotspot/share/code/dependencyContext.cpp	Tue Dec 04 15:47:05 2018 +0100
+++ b/src/hotspot/share/code/dependencyContext.cpp	Tue Dec 04 17:14:11 2018 +0100
@@ -35,8 +35,9 @@
 PerfCounter* DependencyContext::_perf_total_buckets_deallocated_count = NULL;
 PerfCounter* DependencyContext::_perf_total_buckets_stale_count       = NULL;
 PerfCounter* DependencyContext::_perf_total_buckets_stale_acc_count   = NULL;
-nmethodBucket* volatile DependencyContext::_purge_list = NULL;
-volatile uint64_t DependencyContext::_cleaning_epoch = 0;
+nmethodBucket* volatile DependencyContext::_purge_list                = NULL;
+volatile uint64_t DependencyContext::_cleaning_epoch                  = 0;
+uint64_t  DependencyContext::_cleaning_epoch_monotonic                = 0;
 
 void dependencyContext_init() {
   DependencyContext::init();
@@ -262,7 +263,7 @@
   return Atomic::sub(1, &_count);
 }
 
-// We use a safepoint counter to track the safepoint counter the last time a given
+// We use a monotonically increasing epoch counter to track the last epoch a given
 // dependency context was cleaned. GC threads claim cleanup tasks by performing
 // a CAS on this value.
 bool DependencyContext::claim_cleanup() {
@@ -311,13 +312,15 @@
 // a purge list to be deleted later.
 void DependencyContext::cleaning_start() {
   assert(SafepointSynchronize::is_at_safepoint(), "must be");
-  uint64_t epoch = SafepointSynchronize::safepoint_counter();
+  uint64_t epoch = ++_cleaning_epoch_monotonic;
   Atomic::store(epoch, &_cleaning_epoch);
 }
 
 // The epilogue marks the end of dependency context cleanup by the GC,
-// and also makes subsequent releases of nmethodBuckets case immediate
-// deletion. It is admitted to end the cleanup in a concurrent phase.
+// and also makes subsequent releases of nmethodBuckets cause immediate
+// deletion. It is okay to delay calling of cleaning_end() to a concurrent
+// phase, subsequent to the safepoint operation in which cleaning_start()
+// was called. That allows dependency contexts to be cleaned concurrently.
 void DependencyContext::cleaning_end() {
   uint64_t epoch = 0;
   Atomic::store(epoch, &_cleaning_epoch);
--- a/src/hotspot/share/code/dependencyContext.hpp	Tue Dec 04 15:47:05 2018 +0100
+++ b/src/hotspot/share/code/dependencyContext.hpp	Tue Dec 04 17:14:11 2018 +0100
@@ -86,11 +86,12 @@
   nmethodBucket* dependencies();
   nmethodBucket* dependencies_not_unloading();
 
-  static PerfCounter* _perf_total_buckets_allocated_count;
-  static PerfCounter* _perf_total_buckets_deallocated_count;
-  static PerfCounter* _perf_total_buckets_stale_count;
-  static PerfCounter* _perf_total_buckets_stale_acc_count;
+  static PerfCounter*            _perf_total_buckets_allocated_count;
+  static PerfCounter*            _perf_total_buckets_deallocated_count;
+  static PerfCounter*            _perf_total_buckets_stale_count;
+  static PerfCounter*            _perf_total_buckets_stale_acc_count;
   static nmethodBucket* volatile _purge_list;
+  static uint64_t                _cleaning_epoch_monotonic;
   static volatile uint64_t       _cleaning_epoch;
 
  public:
--- a/src/hotspot/share/prims/methodHandles.cpp	Tue Dec 04 15:47:05 2018 +0100
+++ b/src/hotspot/share/prims/methodHandles.cpp	Tue Dec 04 17:14:11 2018 +0100
@@ -1086,8 +1086,6 @@
 }
 
 void MethodHandles::clean_dependency_context(oop call_site) {
-  assert_locked_or_safepoint(CodeCache_lock);
-
   oop context = java_lang_invoke_CallSite::context_no_keepalive(call_site);
   DependencyContext deps = java_lang_invoke_MethodHandleNatives_CallSiteContext::vmdependencies(context);
   deps.clean_unloading_dependents();