8259486: Replace PreserveExceptionMark with implementation for CautiouslyPreserveExceptionMark

coleenp · coleenp · commit 535f2da5e2f5 · 2021-01-13T14:32:52.000Z
Reviewed-by: dholmes, sspitsyn
diff --git a/src/hotspot/cpu/arm/methodHandles_arm.cpp b/src/hotspot/cpu/arm/methodHandles_arm.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2021, 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
@@ -511,14 +511,15 @@ void trace_method_handle_stub(const char* adaptername,
     {
       // dump last frame (from JavaThread::print_frame_layout)
 
-      // Note: code is robust but the dumped informationm may not be
+      // Note: code is robust but the dumped information may not be
       // 100% correct, particularly with respect to the dumped
       // "unextended_sp". Getting it right for all trace_method_handle
       // call paths is not worth the complexity/risk. The correct slot
       // will be identified by *Rsender_sp anyway in the dump.
       JavaThread* p = JavaThread::active();
 
-      PRESERVE_EXCEPTION_MARK;
+      // may not be needed by safer and unexpensive here
+      PreserveExceptionMark pem(Thread::current());
       FrameValues values;
 
       intptr_t* dump_fp = (intptr_t *) saved_bp;
diff --git a/src/hotspot/cpu/ppc/methodHandles_ppc.cpp b/src/hotspot/cpu/ppc/methodHandles_ppc.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2012, 2017 SAP SE. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -519,7 +519,8 @@ void trace_method_handle_stub(const char* adaptername,
 
       JavaThread* p = JavaThread::active();
 
-      PRESERVE_EXCEPTION_MARK; // may not be needed by safer and unexpensive here
+      // may not be needed by safer and unexpensive here
+      PreserveExceptionMark pem(Thread::current());
       FrameValues values;
 
       // Note: We want to allow trace_method_handle from any call site.
diff --git a/src/hotspot/cpu/s390/methodHandles_s390.cpp b/src/hotspot/cpu/s390/methodHandles_s390.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2016, 2017 SAP SE. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -568,7 +568,8 @@ void trace_method_handle_stub(const char* adaptername,
     LogStream ls(lt);
     JavaThread* p = JavaThread::active();
 
-    PRESERVE_EXCEPTION_MARK; // May not be needed by safer and unexpensive here.
+    // may not be needed by safer and unexpensive here
+    PreserveExceptionMark pem(Thread::current());
     FrameValues values;
 
     // Note: We want to allow trace_method_handle from any call site.
diff --git a/src/hotspot/cpu/x86/methodHandles_x86.cpp b/src/hotspot/cpu/x86/methodHandles_x86.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, 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
@@ -549,7 +549,8 @@ void trace_method_handle_stub(const char* adaptername,
 
       JavaThread* p = JavaThread::active();
 
-      PRESERVE_EXCEPTION_MARK; // may not be needed but safer and inexpensive here
+      // may not be needed by safer and unexpensive here
+      PreserveExceptionMark pem(Thread::current());
       FrameValues values;
 
       // Current C frame
diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, 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
@@ -1997,7 +1997,7 @@ oop java_lang_Throwable::message(oop throwable) {
 
 // Return Symbol for detailed_message or NULL
 Symbol* java_lang_Throwable::detail_message(oop throwable) {
-  PRESERVE_EXCEPTION_MARK;  // Keep original exception
+  PreserveExceptionMark pm(Thread::current());
   oop detailed_message = java_lang_Throwable::message(throwable);
   if (detailed_message != NULL) {
     return java_lang_String::as_symbol(detailed_message);
@@ -2187,7 +2187,7 @@ class BacktraceBuilder: public StackObj {
     _index++;
   }
 
-  void set_has_hidden_top_frame(TRAPS) {
+  void set_has_hidden_top_frame() {
     if (!_has_hidden_top_frame) {
       // It would be nice to add java/lang/Boolean::TRUE here
       // to indicate that this backtrace has a hidden top frame.
@@ -2525,7 +2525,7 @@ void java_lang_Throwable::fill_in_stack_trace(Handle throwable, const methodHand
       if (skip_hidden) {
         if (total_count == 0) {
           // The top frame will be hidden from the stack trace.
-          bt.set_has_hidden_top_frame(CHECK);
+          bt.set_has_hidden_top_frame();
         }
         continue;
       }
@@ -2552,11 +2552,12 @@ void java_lang_Throwable::fill_in_stack_trace(Handle throwable, const methodHand
     return;
   }
 
-  PRESERVE_EXCEPTION_MARK;
+  JavaThread* THREAD = JavaThread::current();
+  PreserveExceptionMark pm(THREAD);
 
-  JavaThread* thread = JavaThread::active();
-  fill_in_stack_trace(throwable, method, thread);
-  // ignore exceptions thrown during stack trace filling
+  fill_in_stack_trace(throwable, method, THREAD);
+  // Ignore exceptions thrown during stack trace filling (OOM) and reinstall the
+  // original exception via the PreserveExceptionMark destructor.
   CLEAR_PENDING_EXCEPTION;
 }
 
diff --git a/src/hotspot/share/jfr/recorder/service/jfrRecorderThread.cpp b/src/hotspot/share/jfr/recorder/service/jfrRecorderThread.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, 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
@@ -113,7 +113,7 @@ bool JfrRecorderThread::start(JfrCheckpointManager* cp_manager, JfrPostBox* post
   remove_thread_args.set_signature(vmSymbols::thread_void_signature());
   remove_thread_args.set_receiver(Universe::system_thread_group());
   remove_thread_args.push_oop(h_thread_oop());
-  CautiouslyPreserveExceptionMark cpe(THREAD);
+  PreserveExceptionMark cpe(THREAD);
   JfrJavaSupport::call_special(&remove_thread_args, THREAD);
   return false;
 }
diff --git a/src/hotspot/share/prims/jvmtiEnter.xsl b/src/hotspot/share/prims/jvmtiEnter.xsl
@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <!--
- Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved.
+ Copyright (c) 2002, 2021, 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
@@ -442,7 +442,7 @@ struct jvmtiInterface_1_ jvmti</xsl:text>
     <xsl:text>debug_only(VMNativeEntryWrapper __vew;)</xsl:text>
     <xsl:if test="count(@callbacksafe)=0 or not(contains(@callbacksafe,'safe'))">
       <xsl:value-of select="$space"/>
-      <xsl:text>CautiouslyPreserveExceptionMark __em(this_thread);</xsl:text>
+      <xsl:text>PreserveExceptionMark __em(this_thread);</xsl:text>
     </xsl:if>
   </xsl:if>
 </xsl:template>
diff --git a/src/hotspot/share/runtime/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, 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
@@ -739,7 +739,7 @@ JRT_LEAF(BasicType, Deoptimization::unpack_frames(JavaThread* thread, int exec_m
   if (VerifyStack) {
     ResourceMark res_mark;
     // Clear pending exception to not break verification code (restored afterwards)
-    PRESERVE_EXCEPTION_MARK;
+    PreserveExceptionMark pm(thread);
 
     thread->validate_frame_layout();
 
diff --git a/src/hotspot/share/runtime/handshake.cpp b/src/hotspot/share/runtime/handshake.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2021, 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
@@ -420,7 +420,7 @@ void HandshakeState::process_by_self() {
 void HandshakeState::process_self_inner() {
   while (should_process()) {
     HandleMark hm(_handshakee);
-    CautiouslyPreserveExceptionMark pem(_handshakee);
+    PreserveExceptionMark pem(_handshakee);
     MutexLocker ml(&_lock, Mutex::_no_safepoint_check_flag);
     HandshakeOperation* op = pop_for_self();
     if (op != NULL) {
diff --git a/src/hotspot/share/runtime/thread.cpp b/src/hotspot/share/runtime/thread.cpp
@@ -1859,12 +1859,6 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
   Handle threadObj(this, this->threadObj());
   assert(threadObj.not_null(), "Java thread object should be created");
 
-  // FIXIT: This code should be moved into else part, when reliable 1.2/1.3 check is in place
-  {
-    EXCEPTION_MARK;
-
-    CLEAR_PENDING_EXCEPTION;
-  }
   if (!destroy_vm) {
     if (uncaught_exception.not_null()) {
       EXCEPTION_MARK;
@@ -3023,7 +3017,7 @@ class PrintAndVerifyOopClosure: public OopClosure {
 // Print or validate the layout of stack frames
 void JavaThread::print_frame_layout(int depth, bool validate_only) {
   ResourceMark rm;
-  PRESERVE_EXCEPTION_MARK;
+  PreserveExceptionMark pm(this);
   FrameValues values;
   int frame_no = 0;
   for (StackFrameStream fst(this, false /* update */, true /* process_frames */); !fst.is_done(); fst.next()) {
diff --git a/src/hotspot/share/utilities/exceptions.cpp b/src/hotspot/share/utilities/exceptions.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2021, 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
@@ -364,7 +364,7 @@ Handle Exceptions::new_exception(Thread* thread, Symbol* name,
     // around the allocation.
     // If we get an exception from the allocation, prefer that to
     // the exception we are trying to build, or the pending exception.
-    // This is sort of like what PRESERVE_EXCEPTION_MARK does, except
+    // This is sort of like what PreserveExceptionMark does, except
     // for the preferencing and the early returns.
     Handle incoming_exception(thread, NULL);
     if (thread->has_pending_exception()) {
diff --git a/src/hotspot/share/utilities/exceptions.hpp b/src/hotspot/share/utilities/exceptions.hpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2021, 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
@@ -343,7 +343,7 @@ class ExceptionMark {
 // pending exception exists upon entering its scope and tests that no pending exception
 // exists when leaving the scope.
 
-// See also preserveException.hpp for PRESERVE_EXCEPTION_MARK macro,
+// See also preserveException.hpp for PreserveExceptionMark
 // which preserves pre-existing exceptions and does not allow new
 // exceptions.
 
diff --git a/src/hotspot/share/utilities/preserveException.cpp b/src/hotspot/share/utilities/preserveException.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2021, 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
@@ -23,59 +23,34 @@
  */
 
 #include "precompiled.hpp"
+#include "memory/resourceArea.hpp"
+#include "oops/oop.hpp"
 #include "runtime/handles.inline.hpp"
 #include "utilities/preserveException.hpp"
 
-// TODO: These three classes should be refactored
-
-PreserveExceptionMark::PreserveExceptionMark(Thread*& thread) {
-  thread     = Thread::current();
-  _thread    = thread;
+PreserveExceptionMark::PreserveExceptionMark(Thread* thread) {
+  _thread = thread;
   _preserved_exception_oop = Handle(thread, _thread->pending_exception());
   _preserved_exception_line = _thread->exception_line();
   _preserved_exception_file = _thread->exception_file();
-  _thread->clear_pending_exception(); // Needed to avoid infinite recursion
+  _thread->clear_pending_exception(); // Pending exceptions are checked in the destructor
 }
 
 
 PreserveExceptionMark::~PreserveExceptionMark() {
   if (_thread->has_pending_exception()) {
     oop exception = _thread->pending_exception();
     _thread->clear_pending_exception(); // Needed to avoid infinite recursion
-    exception->print();
-    fatal("PreserveExceptionMark destructor expects no pending exceptions");
-  }
-  if (_preserved_exception_oop() != NULL) {
-    _thread->set_pending_exception(_preserved_exception_oop(), _preserved_exception_file, _preserved_exception_line);
+    ResourceMark rm(_thread);
+    assert(false, "PreserveExceptionMark destructor expects no pending exceptions %s",
+                  exception->print_string());
   }
-}
-
-
-// This code is cloned from PreserveExceptionMark, except that:
-//   returned pending exceptions do not cause a crash.
-//   thread is passed in, not set (not a reference parameter)
-//   and bug 6431341 has been addressed.
-
-CautiouslyPreserveExceptionMark::CautiouslyPreserveExceptionMark(Thread* thread) {
-  _thread    = thread;
-  _preserved_exception_oop = Handle(thread, _thread->pending_exception());
-  _preserved_exception_line = _thread->exception_line();
-  _preserved_exception_file = _thread->exception_file();
-  _thread->clear_pending_exception(); // Pending exceptions are checked in the destructor
-}
 
-
-CautiouslyPreserveExceptionMark::~CautiouslyPreserveExceptionMark() {
-  assert(!_thread->has_pending_exception(), "unexpected exception generated");
-  if (_thread->has_pending_exception()) {
-    _thread->clear_pending_exception();
-  }
   if (_preserved_exception_oop() != NULL) {
     _thread->set_pending_exception(_preserved_exception_oop(), _preserved_exception_file, _preserved_exception_line);
   }
 }
 
-
 void WeakPreserveExceptionMark::preserve() {
   _preserved_exception_oop = Handle(_thread, _thread->pending_exception());
   _preserved_exception_line = _thread->exception_line();
diff --git a/src/hotspot/share/utilities/preserveException.hpp b/src/hotspot/share/utilities/preserveException.hpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2021, 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
@@ -26,7 +26,6 @@
 #define SHARE_UTILITIES_PRESERVEEXCEPTION_HPP
 
 #include "runtime/handles.hpp"
-#include "runtime/thread.hpp"
 
 // This file provides more support for exception handling; see also exceptions.hpp
 class PreserveExceptionMark {
@@ -37,27 +36,11 @@ class PreserveExceptionMark {
   const char* _preserved_exception_file;
 
  public:
-  PreserveExceptionMark(Thread*& thread);
+  PreserveExceptionMark(Thread* thread);
   ~PreserveExceptionMark();
 };
 
 
-// This is a clone of PreserveExceptionMark which asserts instead
-// of failing when what it wraps generates a pending exception.
-// It also addresses bug 6431341.
-class CautiouslyPreserveExceptionMark {
- private:
-  Thread*     _thread;
-  Handle      _preserved_exception_oop;
-  int         _preserved_exception_line;
-  const char* _preserved_exception_file;
-
- public:
-  CautiouslyPreserveExceptionMark(Thread* thread);
-  ~CautiouslyPreserveExceptionMark();
-};
-
-
 // Like PreserveExceptionMark but allows new exceptions to be generated in
 // the body of the mark. If a new exception is generated then the original one
 // is discarded.
@@ -72,8 +55,8 @@ class WeakPreserveExceptionMark {
   void        restore();
 
   public:
-    WeakPreserveExceptionMark(Thread* pThread) :  _thread(pThread), _preserved_exception_oop()  {
-      if (pThread->has_pending_exception()) {
+    WeakPreserveExceptionMark(Thread* thread) :  _thread(thread) {
+      if (thread->has_pending_exception()) {
         preserve();
       }
     }
@@ -84,10 +67,4 @@ class WeakPreserveExceptionMark {
     }
 };
 
-
-
-// use global exception mark when allowing pending exception to be set and
-// saving and restoring them
-#define PRESERVE_EXCEPTION_MARK                    Thread* THREAD; PreserveExceptionMark __em(THREAD);
-
 #endif // SHARE_UTILITIES_PRESERVEEXCEPTION_HPP
