8254824: SignalHandlerMark have no purpose

robehn · robehn · commit 011dd0d8fa4e · 2020-10-19T06:28:47.000Z
Reviewed-by: stuefe, shade, dholmes, coleenp
diff --git a/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp b/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp
@@ -177,8 +177,6 @@ JVM_handle_aix_signal(int sig, siginfo_t* info, void* ucVoid, int abort_if_unrec
 
   Thread* t = Thread::current_or_null_safe();
 
-  SignalHandlerMark shm(t);
-
   // Note: it's not uncommon that JNI code uses signal/sigset to install
   // then restore certain signal handler (e.g. to temporarily block SIGPIPE,
   // or have a SIGILL handler when detecting CPU type). When that happens,
diff --git a/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp b/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp
@@ -401,12 +401,11 @@ JVM_handle_bsd_signal(int sig,
 
   Thread* t = Thread::current_or_null_safe();
 
-  // Must do this before SignalHandlerMark, if crash protection installed we will longjmp away
-  // (no destructors can be run)
+  // If crash protection is installed we may longjmp away and no destructors
+  // for objects in this scope will be run.
+  // So don't use any RAII utilities before crash protection is checked.
   os::ThreadCrashProtection::check_crash_protection(sig, t);
 
-  SignalHandlerMark shm(t);
-
   // Note: it's not uncommon that JNI code uses signal/sigset to install
   // then restore certain signal handler (e.g. to temporarily block SIGPIPE,
   // or have a SIGILL handler when detecting CPU type). When that happens,
diff --git a/src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp b/src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp
@@ -124,8 +124,6 @@ JVM_handle_bsd_signal(int sig,
 
   Thread* t = Thread::current_or_null_safe();
 
-  SignalHandlerMark shm(t);
-
   // handle SafeFetch faults
   if (sig == SIGSEGV || sig == SIGBUS) {
     sigjmp_buf* const pjb = get_jmp_buf_for_continuation();
diff --git a/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp b/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp
@@ -173,12 +173,11 @@ JVM_handle_linux_signal(int sig,
 
   Thread* t = Thread::current_or_null_safe();
 
-  // Must do this before SignalHandlerMark, if crash protection installed we will longjmp away
-  // (no destructors can be run)
+  // If crash protection is installed we may longjmp away and no destructors
+  // for objects in this scope will be run.
+  // So don't use any RAII utilities before crash protection is checked.
   os::ThreadCrashProtection::check_crash_protection(sig, t);
 
-  SignalHandlerMark shm(t);
-
   // Note: it's not uncommon that JNI code uses signal/sigset to install
   // then restore certain signal handler (e.g. to temporarily block SIGPIPE,
   // or have a SIGILL handler when detecting CPU type). When that happens,
diff --git a/src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp b/src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp
@@ -249,12 +249,11 @@ extern "C" int JVM_handle_linux_signal(int sig, siginfo_t* info,
 
   Thread* t = Thread::current_or_null_safe();
 
-  // Must do this before SignalHandlerMark, if crash protection installed we will longjmp away
-  // (no destructors can be run)
+  // If crash protection is installed we may longjmp away and no destructors
+  // for objects in this scope will be run.
+  // So don't use any RAII utilities before crash protection is checked.
   os::ThreadCrashProtection::check_crash_protection(sig, t);
 
-  SignalHandlerMark shm(t);
-
   if (sig == SIGILL &&
       ((info->si_addr == (caddr_t)check_simd_fault_instr)
        || info->si_addr == (caddr_t)check_vfp_fault_instr
diff --git a/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp b/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp
@@ -199,8 +199,6 @@ JVM_handle_linux_signal(int sig,
 
   Thread* t = Thread::current_or_null_safe();
 
-  SignalHandlerMark shm(t);
-
   // Note: it's not uncommon that JNI code uses signal/sigset to install
   // then restore certain signal handler (e.g. to temporarily block SIGPIPE,
   // or have a SIGILL handler when detecting CPU type). When that happens,
diff --git a/src/hotspot/os_cpu/linux_s390/os_linux_s390.cpp b/src/hotspot/os_cpu/linux_s390/os_linux_s390.cpp
@@ -215,12 +215,11 @@ JVM_handle_linux_signal(int sig,
 
   Thread* t = Thread::current_or_null_safe();
 
-  // Must do this before SignalHandlerMark, if crash protection installed we will longjmp away
-  // (no destructors can be run).
+  // If crash protection is installed we may longjmp away and no destructors
+  // for objects in this scope will be run.
+  // So don't use any RAII utilities before crash protection is checked.
   os::ThreadCrashProtection::check_crash_protection(sig, t);
 
-  SignalHandlerMark shm(t);
-
   // Note: it's not uncommon that JNI code uses signal/sigset to install
   // then restore certain signal handler (e.g. to temporarily block SIGPIPE,
   // or have a SIGILL handler when detecting CPU type). When that happens,
diff --git a/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp b/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp
@@ -209,12 +209,11 @@ JVM_handle_linux_signal(int sig,
 
   Thread* t = Thread::current_or_null_safe();
 
-  // Must do this before SignalHandlerMark, if crash protection installed we will longjmp away
-  // (no destructors can be run)
+  // If crash protection is installed we may longjmp away and no destructors
+  // for objects in this scope will be run.
+  // So don't use any RAII utilities before crash protection is checked.
   os::ThreadCrashProtection::check_crash_protection(sig, t);
 
-  SignalHandlerMark shm(t);
-
   // Note: it's not uncommon that JNI code uses signal/sigset to install
   // then restore certain signal handler (e.g. to temporarily block SIGPIPE,
   // or have a SIGILL handler when detecting CPU type). When that happens,
diff --git a/src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp b/src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp
@@ -120,8 +120,6 @@ JVM_handle_linux_signal(int sig,
 
   Thread* t = Thread::current_or_null_safe();
 
-  SignalHandlerMark shm(t);
-
   // handle SafeFetch faults
   if (sig == SIGSEGV || sig == SIGBUS) {
     sigjmp_buf* const pjb = get_jmp_buf_for_continuation();
diff --git a/src/hotspot/share/runtime/thread.cpp b/src/hotspot/share/runtime/thread.cpp
@@ -260,7 +260,6 @@ Thread::Thread() {
   _current_pending_monitor_is_from_java = true;
   _current_waiting_monitor = NULL;
   _current_pending_raw_monitor = NULL;
-  _num_nested_signal = 0;
   om_free_list = NULL;
   om_free_count = 0;
   om_free_provision = 32;
diff --git a/src/hotspot/share/runtime/thread.hpp b/src/hotspot/share/runtime/thread.hpp
@@ -311,15 +311,9 @@ class Thread: public ThreadShadow {
   volatile uint32_t _suspend_flags;
 
  private:
-  int _num_nested_signal;
-
   DEBUG_ONLY(bool _suspendible_thread;)
 
  public:
-  void enter_signal_handler() { _num_nested_signal++; }
-  void leave_signal_handler() { _num_nested_signal--; }
-  bool is_inside_signal_handler() const { return _num_nested_signal > 0; }
-
   // Determines if a heap allocation failure will be retried
   // (e.g., by deoptimizing and re-executing in the interpreter).
   // In this case, the failed allocation must raise
@@ -2146,20 +2140,6 @@ class Threads: AllStatic {
   struct Test;                  // For private gtest access.
 };
 
-class SignalHandlerMark: public StackObj {
- private:
-  Thread* _thread;
- public:
-  SignalHandlerMark(Thread* t) {
-    _thread = t;
-    if (_thread) _thread->enter_signal_handler();
-  }
-  ~SignalHandlerMark() {
-    if (_thread) _thread->leave_signal_handler();
-    _thread = NULL;
-  }
-};
-
 class UnlockFlagSaver {
   private:
     JavaThread* _thread;
