JDK-8024346 : ~CautiouslyPreserveExceptionMark - assert(!_thread->has_pending_exception()) failed: unexpected exception generated
  • Type: Bug
  • Component: hotspot
  • Sub-Component: jvmti
  • Affected Version: hs25
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2013-09-05
  • Updated: 2013-09-24
  • Resolved: 2013-09-14
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 Other
8Fixed hs25Fixed
Description
#  Internal Error (/localhome/stefank/hg/hsx-gc/src/share/vm/utilities/preserveException.cpp:69), pid=16936, tid=140168939996928
#  assert(!_thread->has_pending_exception()) failed: unexpected exception generated

Stack: [0x00007f7b9fd08000,0x00007f7b9fe09000],  sp=0x00007f7b9fe07230,  free space=1020k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0xe29918]  VMError::report_and_die()+0x158
V  [libjvm.so+0x698063]  report_vm_error(char const*, int, char const*, char const*)+0x83
V  [libjvm.so+0xc61f25]  CautiouslyPreserveExceptionMark::~CautiouslyPreserveExceptionMark()+0x35
V  [libjvm.so+0x9a82ee]  jvmti_RedefineClasses+0x1ce

Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
j  runtime.ParallelClassLoading.shared.ClassLoadingController.redefineClasses(Ljava/lang/ClassLoader;Lruntime/ParallelClassLoading/shared/ClassLoadingController$ByteCodeForRedefinitionHandler;)I+0
j  runtime.ParallelClassLoading.shared.ClassLoadingController.startLoadingIterator()Z+806
j  runtime.ParallelClassLoading.shared.ClassLoadingController.runIt([Ljava/lang/String;Ljava/io/PrintStream;)I+9
j  runtime.ParallelClassLoading.shared.MLetController.run([Ljava/lang/String;Ljava/io/PrintStream;)I+9
j  runtime.ParallelClassLoading.shared.MLetController.main([Ljava/lang/String;)V+4
v  ~StubRoutines::call_stub
Comments
It is Ok. It is useful anyway, like I have another pair of eyes that is double-checking everything. :)
11-09-2013
Thanks for clarifying the context. I had misunderstood what might happen prior to the exception being posted and so was concerned about the first invariant. I also now see that this pattern is used throughout the code. Sorry for the noise.
11-09-2013
The first invariant is preserved, I do not see any issue with it yet. In a HAS_PENDING_EXCEPTION case the code returns from the load_new_class_versions() with a non-JVMTI_ERROR_NONE result. The doit_prologue() returns false in such a case. The VM operation is cancelled and so, new class versions are not installed.
10-09-2013
The suggested patch is only trying to set the correct return code for OOME. But it does nothing to establish the first invariant as far as I can see. The OOME needs to be detected closer to where it occurs to be correctly handled in my opinion.
10-09-2013
The CautiouslyPreserveExceptionMark is in the jvmtiEnter.cpp which is auto-generated from the jvmti.xml. It is to catch the exceptions that are not handled properly which is exactly our case (see stack trace). It is a wrong place to handle the exception. The patch suggested by Stefan in general is going to work as necessary to provide the invariants listed above. Note, there are 6-7 more places like this in the VM_RedefineClasses. Probably, having just an assert to guard against res==JVMTI_ERROR_NONE is not enough. It is going to work correctly for current code as the exception we face is an OOME. But the code might change in the future, so a better guard would be good. Before "return res;" add something like this: if (res == JVMTI_ERROR_NONE) res = JVMTI_ERROR_INTERNAL;
10-09-2013
I think this is the wrong place to fix this problem. If an error occurs there are two important things that have to happen according to the spec: 1. If any error code is returned other than JVMTI_ERROR_NONE, none of the classes to be redefined will have a new definition installed. 2. The appropriate error code has to be set. So it would seem to me that whatever code is taking care of these two issues is the code that should be clearing the exception. Hence the ExceptionMark in the existing code is correct because it is trapping the fact that we really should not have allowed any exceptions to remain pending this long. We need to see what exceptions are being propagated and ensure they are trapped and cleared at the appropriate point so that #1 and #2 are fulfilled as required by the spec.
09-09-2013
A suggested fix from Stefan: diff --git a/src/share/vm/prims/jvmtiRedefineClasses.cpp b/src/share/vm/prims/jvmtiRedefineClasses.cpp --- a/src/share/vm/prims/jvmtiRedefineClasses.cpp +++ b/src/share/vm/prims/jvmtiRedefineClasses.cpp @@ -1072,8 +1074,17 @@ } res = merge_cp_and_rewrite(the_class, scratch_class, THREAD); - if (res != JVMTI_ERROR_NONE) { - return res; + + if (HAS_PENDING_EXCEPTION) { + Symbol* ex_name = PENDING_EXCEPTION->klass()->name(); + // RC_TRACE_WITH_THREAD macro has an embedded ResourceMark + CLEAR_PENDING_EXCEPTION; + if (ex_name == vmSymbols::java_lang_OutOfMemoryError()) { + return JVMTI_ERROR_OUT_OF_MEMORY; + } else { + assert(res != JVMTI_ERROR_NONE, "Must be"); + return res; + } } if (VerifyMergedCPBytecodes) {
06-09-2013
Stefan, I think you correctly identified the buggy spot. Thanks.
06-09-2013
I think this is the buggy path: jvmtiError VM_RedefineClasses::load_new_class_versions(TRAPS) { ... res = merge_cp_and_rewrite(the_class, scratch_class, THREAD); if (res != JVMTI_ERROR_NONE) { return res; } We don't check and clear pending exceptions when returning.
06-09-2013