8273143: Transition to _thread_in_vm when handling a polling page exception

Reviewed-by: rehn, dcubed, coleenp, rrich

src/hotspot/share/interpreter/zero/bytecodeInterpreter.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2022, 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
@@ -99,6 +99,9 @@
 #undef PREFETCH_OPCCODE
 #define PREFETCH_OPCCODE
 
+JRT_ENTRY(void, at_safepoint(JavaThread* current)) {}
+JRT_END
+
 /*
   Interpreter safepoint: it is expected that the interpreter will have no live
   handles of its own creation live at an interpreter safepoint. Therefore we
@@ -107,12 +110,10 @@
   There really shouldn't be any handles remaining to trash but this is cheap
   in relation to a safepoint.
 */
-#define RETURN_SAFEPOINT                                                                                  \
+#define RETURN_SAFEPOINT                                    \
-    if (SafepointMechanism::should_process(THREAD)) {                                                     \
+    if (SafepointMechanism::should_process(THREAD)) {       \
-      HandleMarkCleaner __hmc(THREAD);                                                                    \
+      CALL_VM(at_safepoint(THREAD), handle_exception);      \
-      CALL_VM(SafepointMechanism::process_if_requested_with_exit_check(THREAD, true /* check asyncs */),  \
+    }
-              handle_exception);                                                                          \
-    }                                                                                                     \
 
 /*
  * VM_JAVA_ERROR - Macro for throwing a java exception from

src/hotspot/share/runtime/handshake.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2022, 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
@@ -488,10 +488,11 @@ void HandshakeState::remove_op(HandshakeOperation* op) {
 bool HandshakeState::process_by_self(bool allow_suspend) {
   assert(Thread::current() == _handshakee, "should call from _handshakee");
   assert(!_handshakee->is_terminated(), "should not be a terminated thread");
-  assert(_handshakee->thread_state() != _thread_blocked, "should not be in a blocked state");
-  assert(_handshakee->thread_state() != _thread_in_native, "should not be in native");
 
-  ThreadInVMForHandshake tivm(_handshakee);
+  _handshakee->frame_anchor()->make_walkable(_handshakee);
+  // Threads shouldn't block if they are in the middle of printing, but...
+  ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id());
+
   // Handshakes cannot safely safepoint.
   // The exception to this rule is the asynchronous suspension handshake.
   // It by-passes the NSV by manually doing the transition.

src/hotspot/share/runtime/interfaceSupport.inline.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2021, Azul Systems, Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -120,29 +120,6 @@ class ThreadStateTransition : public StackObj {
   }
 };
 
-class ThreadInVMForHandshake : public ThreadStateTransition {
-  const JavaThreadState _original_state;
- public:
-  ThreadInVMForHandshake(JavaThread* thread) : ThreadStateTransition(thread),
-      _original_state(thread->thread_state()) {
-
-    if (thread->has_last_Java_frame()) {
-      thread->frame_anchor()->make_walkable(thread);
-    }
-
-    thread->set_thread_state(_thread_in_vm);
-
-    // Threads shouldn't block if they are in the middle of printing, but...
-    ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id());
-  }
-
-  ~ThreadInVMForHandshake() {
-    assert(_thread->thread_state() == _thread_in_vm, "should only call when leaving VM after handshake");
-    _thread->set_thread_state(_original_state);
-  }
-
-};
-
 class ThreadInVMfromJava : public ThreadStateTransition {
   bool _check_asyncs;
  public:
@@ -222,7 +199,7 @@ class ThreadBlockInVMPreprocess : public ThreadStateTransition {
   PRE_PROC& _pr;
   bool _allow_suspend;
  public:
-  ThreadBlockInVMPreprocess(JavaThread* thread, PRE_PROC& pr = emptyOp, bool allow_suspend = false)
+  ThreadBlockInVMPreprocess(JavaThread* thread, PRE_PROC& pr, bool allow_suspend = false)
     : ThreadStateTransition(thread), _pr(pr), _allow_suspend(allow_suspend) {
     transition_from_vm(thread, _thread_blocked);
   }
@@ -236,11 +213,15 @@ class ThreadBlockInVMPreprocess : public ThreadStateTransition {
       SafepointMechanism::process_if_requested(_thread, _allow_suspend);
     }
   }
-
-  static void emptyOp(JavaThread* current) {}
 };
 
-typedef ThreadBlockInVMPreprocess<> ThreadBlockInVM;
+class ThreadBlockInVM  : public ThreadBlockInVMPreprocess<> {
+ public:
+  ThreadBlockInVM(JavaThread* thread, bool allow_suspend = false)
+    : ThreadBlockInVMPreprocess(thread, emptyOp, allow_suspend) {}
+ private:
+  static void emptyOp(JavaThread* current) {}
+};
 
 
 // Debug class instantiated in JRT_ENTRY macro.

src/hotspot/share/runtime/safepoint.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2022, 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
@@ -754,6 +754,7 @@ void SafepointSynchronize::block(JavaThread *thread) {
 
 void SafepointSynchronize::handle_polling_page_exception(JavaThread *thread) {
   assert(thread->thread_state() == _thread_in_Java, "should come from Java code");
+  thread->set_thread_state(_thread_in_vm);
 
   // Enable WXWrite: the function is called implicitly from java code.
   MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXWrite, thread));
@@ -765,6 +766,8 @@ void SafepointSynchronize::handle_polling_page_exception(JavaThread *thread) {
   ThreadSafepointState* state = thread->safepoint_state();
 
   state->handle_polling_page_exception();
+
+  thread->set_thread_state(_thread_in_Java);
 }
 
 
@@ -970,7 +973,6 @@ void ThreadSafepointState::handle_polling_page_exception() {
     // If we have a pending async exception deoptimize the frame
     // as otherwise we may never deliver it.
     if (self->has_async_exception_condition()) {
-      ThreadInVMfromJava __tiv(self, false /* check asyncs */);
       Deoptimization::deoptimize_frame(self, caller_fr.id());
     }
 

src/hotspot/share/runtime/safepoint.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2022, 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
@@ -126,10 +126,6 @@ class SafepointSynchronize : AllStatic {
                                     JavaThread *thread,
                                     uint64_t safepoint_count);
 
-  static bool is_a_block_safe_state(JavaThreadState state) {
-    // Check that we have a valid thread_state before blocking for safepoints
-    return state == _thread_in_vm || state == _thread_in_Java;
-  }
   // Called when a thread voluntarily blocks
   static void block(JavaThread *thread);
 

src/hotspot/share/runtime/safepointMechanism.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2022, 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
@@ -119,7 +119,7 @@ void SafepointMechanism::process(JavaThread *thread, bool allow_suspend) {
   bool need_rechecking;
   do {
     JavaThreadState state = thread->thread_state();
-    guarantee(SafepointSynchronize::is_a_block_safe_state(state), "Illegal threadstate encountered: %d", state);
+    guarantee(state == _thread_in_vm, "Illegal threadstate encountered: %d", state);
     if (global_poll()) {
       // Any load in ::block() must not pass the global poll load.
       // Otherwise we might load an old safepoint counter (for example).

src/hotspot/share/runtime/thread.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2021, Azul Systems, Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -1646,24 +1646,10 @@ void JavaThread::check_and_handle_async_exceptions() {
 
     // We may be at method entry which requires we save the do-not-unlock flag.
     UnlockFlagSaver fs(this);
-    switch (thread_state()) {
+    Exceptions::throw_unsafe_access_internal_error(this, __FILE__, __LINE__, "a fault occurred in an unsafe memory access operation");
-    case _thread_in_vm: {
+    // We might have blocked in a ThreadBlockInVM wrapper in the call above so make sure we process pending
-      JavaThread* THREAD = this;
+    // suspend requests and object reallocation operations if any since we might be going to Java after this.
-      Exceptions::throw_unsafe_access_internal_error(THREAD, __FILE__, __LINE__, "a fault occurred in an unsafe memory access operation");
+    SafepointMechanism::process_if_requested_with_exit_check(this, true /* check asyncs */);
-      // We might have blocked in a ThreadBlockInVM wrapper in the call above so make sure we process pending
-      // suspend requests and object reallocation operations if any since we might be going to Java after this.
-      SafepointMechanism::process_if_requested_with_exit_check(this, true /* check asyncs */);
-      return;
-    }
-    case _thread_in_Java: {
-      ThreadInVMfromJava tiv(this);
-      JavaThread* THREAD = this;
-      Exceptions::throw_unsafe_access_internal_error(THREAD, __FILE__, __LINE__, "a fault occurred in an unsafe memory access operation in compiled Java code");
-      return;
-    }
-    default:
-      ShouldNotReachHere();
-    }
   }
 }
 
@@ -1770,27 +1756,19 @@ bool JavaThread::java_resume() {
 }
 
 // Wait for another thread to perform object reallocation and relocking on behalf of
-// this thread.
+// this thread. The current thread is required to change to _thread_blocked in order
-// Raw thread state transition to _thread_blocked and back again to the original
+// to be seen to be safepoint/handshake safe whilst suspended and only after becoming
-// state before returning are performed. The current thread is required to
+// handshake safe, the other thread can complete the handshake used to synchronize
-// change to _thread_blocked in order to be seen to be safepoint/handshake safe
+// with this thread and then perform the reallocation and relocking.
-// whilst suspended and only after becoming handshake safe, the other thread can
+// See EscapeBarrier::sync_and_suspend_*()
-// complete the handshake used to synchronize with this thread and then perform
-// the reallocation and relocking. We cannot use the thread state transition
-// helpers because we arrive here in various states and also because the helpers
-// indirectly call this method.  After leaving _thread_blocked we have to check
-// for safepoint/handshake, except if _thread_in_native. The thread is safe
-// without blocking then. Allowed states are enumerated in
-// SafepointSynchronize::block(). See also EscapeBarrier::sync_and_suspend_*()
 
 void JavaThread::wait_for_object_deoptimization() {
   assert(!has_last_Java_frame() || frame_anchor()->walkable(), "should have walkable stack");
   assert(this == Thread::current(), "invariant");
-  JavaThreadState state = thread_state();
 
   bool spin_wait = os::is_MP();
   do {
-    set_thread_state(_thread_blocked);
+    ThreadBlockInVM tbivm(this, true /* allow_suspend */);
     // Wait for object deoptimization if requested.
     if (spin_wait) {
       // A single deoptimization is typically very short. Microbenchmarks
@@ -1808,14 +1786,6 @@ void JavaThread::wait_for_object_deoptimization() {
         ml.wait();
       }
     }
-    // The current thread could have been suspended again. We have to check for
-    // suspend after restoring the saved state. Without this the current thread
-    // might return to _thread_in_Java and execute bytecode.
-    set_thread_state_fence(state);
-
-    if (state != _thread_in_native) {
-      SafepointMechanism::process_if_requested(this);
-    }
     // A handshake for obj. deoptimization suspend could have been processed so
     // we must check after processing.
   } while (is_obj_deopt_suspend());