8214936: assert(_needs_refill == 0) failed: Forgot to handle a failed IC transition requiring IC stubs

Reviewed-by: kvn, thartmann, pliden

src/hotspot/share/code/compiledMethod.cpp

@@ -556,6 +556,7 @@ bool CompiledMethod::unload_nmethod_caches(bool unloading_occurred) {
 
 void CompiledMethod::cleanup_inline_caches(bool clean_all) {
   for (;;) {
+    ICRefillVerifier ic_refill_verifier;
     { CompiledICLocker ic_locker(this);
       if (cleanup_inline_caches_impl(false, clean_all)) {
         return;

src/hotspot/share/code/icBuffer.cpp

@@ -38,6 +38,7 @@
 #include "runtime/handles.inline.hpp"
 #include "runtime/mutexLocker.hpp"
 #include "runtime/stubRoutines.hpp"
+#include "runtime/thread.hpp"
 
 DEF_STUB_INTERFACE(ICStub);
 
@@ -45,7 +46,40 @@ StubQueue* InlineCacheBuffer::_buffer    = NULL;
 
 CompiledICHolder* InlineCacheBuffer::_pending_released = NULL;
 int InlineCacheBuffer::_pending_count = 0;
-DEBUG_ONLY(volatile int InlineCacheBuffer::_needs_refill = 0;)
+
+#ifdef ASSERT
+ICRefillVerifier::ICRefillVerifier()
+  : _refill_requested(false),
+    _refill_remembered(false)
+{
+  Thread* thread = Thread::current();
+  assert(thread->missed_ic_stub_refill_mark() == NULL, "nesting not supported");
+  thread->set_missed_ic_stub_refill_mark(this);
+}
+
+ICRefillVerifier::~ICRefillVerifier() {
+  assert(!_refill_requested || _refill_remembered,
+         "Forgot to refill IC stubs after failed IC transition");
+  Thread::current()->set_missed_ic_stub_refill_mark(NULL);
+}
+
+ICRefillVerifierMark::ICRefillVerifierMark(ICRefillVerifier* verifier) {
+  Thread* thread = Thread::current();
+  assert(thread->missed_ic_stub_refill_mark() == NULL, "nesting not supported");
+  thread->set_missed_ic_stub_refill_mark(this);
+}
+
+ICRefillVerifierMark::~ICRefillVerifierMark() {
+  Thread::current()->set_missed_ic_stub_refill_mark(NULL);
+}
+
+static ICRefillVerifier* current_ic_refill_verifier() {
+  Thread* current = Thread::current();
+  ICRefillVerifier* verifier = reinterpret_cast<ICRefillVerifier*>(current->missed_ic_stub_refill_mark());
+  assert(verifier != NULL, "need a verifier for safety");
+  return verifier;
+}
+#endif
 
 void ICStub::finalize() {
   if (!is_empty()) {
@@ -117,7 +151,10 @@ ICStub* InlineCacheBuffer::new_ic_stub() {
 
 
 void InlineCacheBuffer::refill_ic_stubs() {
-  DEBUG_ONLY(Atomic::store(0, &_needs_refill));
+#ifdef ASSERT
+  ICRefillVerifier* verifier = current_ic_refill_verifier();
+  verifier->request_remembered();
+#endif
   // we ran out of inline cache buffer space; must enter safepoint.
   // We do this by forcing a safepoint
   EXCEPTION_MARK;
@@ -135,8 +172,6 @@ void InlineCacheBuffer::refill_ic_stubs() {
 
 
 void InlineCacheBuffer::update_inline_caches() {
-  assert(_needs_refill == 0,
-         "Forgot to handle a failed IC transition requiring IC stubs");
   if (buffer()->number_of_stubs() > 0) {
     if (TraceICBuffer) {
       tty->print_cr("[updating inline caches with %d stubs]", buffer()->number_of_stubs());
@@ -161,7 +196,6 @@ void InlineCacheBuffer_init() {
   InlineCacheBuffer::initialize();
 }
 
-
 bool InlineCacheBuffer::create_transition_stub(CompiledIC *ic, void* cached_value, address entry) {
   assert(!SafepointSynchronize::is_at_safepoint(), "should not be called during a safepoint");
   assert(CompiledICLocker::is_safe(ic->instruction_address()), "mt unsafe call");
@@ -173,7 +207,10 @@ bool InlineCacheBuffer::create_transition_stub(CompiledIC *ic, void* cached_valu
   // allocate and initialize new "out-of-line" inline-cache
   ICStub* ic_stub = new_ic_stub();
   if (ic_stub == NULL) {
-    DEBUG_ONLY(Atomic::inc(&_needs_refill));
+#ifdef ASSERT
+    ICRefillVerifier* verifier = current_ic_refill_verifier();
+    verifier->request_refill();
+#endif
     return false;
   }
 

src/hotspot/share/code/icBuffer.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -29,7 +29,9 @@
 #include "code/stubs.hpp"
 #include "interpreter/bytecodes.hpp"
 #include "memory/allocation.hpp"
+#include "runtime/safepointVerifiers.hpp"
 #include "utilities/align.hpp"
+#include "utilities/debug.hpp"
 #include "utilities/macros.hpp"
 
 //
@@ -93,6 +95,43 @@ inline ICStub* ICStub_from_destination_address(address destination_address) {
   return stub;
 }
 
+#ifdef ASSERT
+// The ICRefillVerifier class is a stack allocated RAII object used to
+// detect if a failed IC transition that required IC stub refilling has
+// been accidentally missed. It is up to the caller to in that case
+// refill IC stubs.
+class ICRefillVerifier: StackObj {
+  bool _refill_requested;
+  bool _refill_remembered;
+
+ public:
+  ICRefillVerifier();
+  ~ICRefillVerifier();
+
+  void request_refill() { _refill_requested = true; }
+  void request_remembered() { _refill_remembered = true; }
+};
+
+// The ICRefillVerifierMark is used to set the thread's current
+// ICRefillVerifier to a provided one. This is useful in particular
+// when transitioning IC stubs in parallel and refilling from the
+// master thread invoking the IC stub transitioning code.
+class ICRefillVerifierMark: StackObj {
+ public:
+  ICRefillVerifierMark(ICRefillVerifier* verifier);
+  ~ICRefillVerifierMark();
+};
+#else
+class ICRefillVerifier: StackObj {
+ public:
+  ICRefillVerifier() {}
+};
+class ICRefillVerifierMark: StackObj {
+ public:
+  ICRefillVerifierMark(ICRefillVerifier* verifier) {}
+};
+#endif
+
 class InlineCacheBuffer: public AllStatic {
  private:
   // friends
@@ -105,8 +144,6 @@ class InlineCacheBuffer: public AllStatic {
   static CompiledICHolder* _pending_released;
   static int _pending_count;
 
-  DEBUG_ONLY(static volatile int _needs_refill;)
-
   static StubQueue* buffer()                         { return _buffer;         }
 
   static ICStub* new_ic_stub();

src/hotspot/share/runtime/sharedRuntime.cpp

@@ -1395,6 +1395,7 @@ methodHandle SharedRuntime::resolve_sub_helper(JavaThread *thread,
   // Patching IC caches may fail if we run out if transition stubs.
   // We refill the ic stubs then and try again.
   for (;;) {
+    ICRefillVerifier ic_refill_verifier;
     bool successful = resolve_sub_helper_internal(callee_method, caller_frame, caller_nm,
                                                   is_virtual, is_optimized, receiver,
                                                   call_info, invoke_code, CHECK_(methodHandle()));
@@ -1603,10 +1604,10 @@ bool SharedRuntime::handle_ic_miss_helper_internal(Handle receiver, CompiledMeth
     // Potential change to megamorphic
 
     bool successful = inline_cache->set_to_megamorphic(&call_info, bc, needs_ic_stub_refill, CHECK_false);
-    if (!successful) {
+    if (needs_ic_stub_refill) {
-      if (!needs_ic_stub_refill) {
       return false;
     }
+    if (!successful) {
       if (!inline_cache->set_to_clean()) {
         needs_ic_stub_refill = true;
         return false;
@@ -1690,6 +1691,7 @@ methodHandle SharedRuntime::handle_ic_miss_helper(JavaThread *thread, TRAPS) {
   CompiledMethod* caller_nm = cb->as_compiled_method();
 
   for (;;) {
+    ICRefillVerifier ic_refill_verifier;
     bool needs_ic_stub_refill = false;
     bool successful = handle_ic_miss_helper_internal(receiver, caller_nm, caller_frame, callee_method,
                                                      bc, call_info, needs_ic_stub_refill, CHECK_(methodHandle()));
@@ -1798,6 +1800,7 @@ methodHandle SharedRuntime::reresolve_call_site(JavaThread *thread, TRAPS) {
       // resolve is only done once.
 
       for (;;) {
+        ICRefillVerifier ic_refill_verifier;
         if (!clear_ic_at_addr(caller_nm, call_addr, is_static_call)) {
           InlineCacheBuffer::refill_ic_stubs();
         } else {

src/hotspot/share/runtime/thread.cpp

@@ -231,6 +231,7 @@ Thread::Thread() {
   set_active_handles(NULL);
   set_free_handle_block(NULL);
   set_last_handle_mark(NULL);
+  DEBUG_ONLY(_missed_ic_stub_refill_mark = NULL);
 
   // This initial value ==> never claimed.
   _oops_do_parity = 0;

src/hotspot/share/runtime/thread.hpp

@@ -328,6 +328,21 @@ class Thread: public ThreadShadow {
   HandleMark* last_handle_mark() const          { return _last_handle_mark; }
  private:
 
+#ifdef ASSERT
+  void* _missed_ic_stub_refill_mark;
+
+ public:
+  void* missed_ic_stub_refill_mark() {
+    return _missed_ic_stub_refill_mark;
+  }
+
+  void set_missed_ic_stub_refill_mark(void* mark) {
+    _missed_ic_stub_refill_mark = mark;
+  }
+#endif
+
+ private:
+
   // debug support for checking if code does allow safepoints or not
   // GC points in the VM can happen because of allocation, invoking a VM operation, or blocking on
   // mutex, or blocking on an object synchronizer (Java locking).