• Type: Bug
• Component: hotspot
• Sub-Component: compiler
• Affected Version: 15,16

• Priority: P3
• Status: Resolved
• Resolution: Fixed

• Submitted: 2020-07-15
• Updated: 2024-11-22
• Resolved: 2020-09-18

JDK 16
16 b17Fixed

Changeset: 73c9088b Author: Jamsheed Mohammed C M <jcm@openjdk.org> Date: 2020-09-18 05:48:14 +0000 URL: https://git.openjdk.java.net/jdk/commit/73c9088b 18-09-2020 Here, I propagate the exception in compilation request path. mdo/method counters can throw metaspace oom, so clearing logic still clears with a comment. deoptimization logic propagate exceptions through exception path for non Uncommon trap case. http://cr.openjdk.java.net/~jcm/8249451/webrev.00/ 05-08-2020 All the non leaf call to VM from Java should have checks for asycn exceptions after it return from the VM. In interpreter case where we use call_VM we have it here [1]. for compiler we use separate mechanism. In short, As you confirmed earlier pending_exception should get checked and handled in predictable manner when it enters Java code, and it just doesn't go for chances. [1] // do the call, remove parameters MacroAssembler::call_VM_leaf_base(entry_point, number_of_arguments); ... // exception could now be pending if (check_exceptions) { // check for pending exceptions (java_thread is set upon return) cmpptr(Address(java_thread, Thread::pending_exception_offset()), (int32_t) NULL_WORD); ... 24-07-2020 Sorry I'm not sure what you mean by that. 24-07-2020 I do not have a full understanding of JDK-8246727 to know why it resulted in the call to the overflow counter check with the async exception pending. It seems a bug to me that this can happen. I would not say such exceptions are "ok to be cleared" rather they should not be occurring in the first place. I think I will re-open JDK-8246727 to investigate how the exception came to be pending such that we proceeded "forward" in the Java execution rather than propagating it. 23-07-2020 > Once pending_exception is detected in vm to Java transition we take control in exception path right(moving to the right handler or rethrowing by unwinding) No. The transition code only moves the pending async exception into the pending exception slot. It is up to the code following the transition to check for the pending exception and either return to the caller, which in turn should check for the exception, or else initiate the dispatch itself. For example, we use a ThreadInVMfromJava transition in JRT_ENTRY, which is invoked from the interpreter via call_VM, which in turn does: // do the call, remove parameters MacroAssembler::call_VM_leaf_base(entry_point, number_of_arguments); ... // exception could now be pending if (check_exceptions) { // check for pending exceptions (java_thread is set upon return) cmpptr(Address(java_thread, Thread::pending_exception_offset()), (int32_t) NULL_WORD); ... 23-07-2020 Thank you [~dholmes] 23-07-2020 Right, and presume there is 1-1 association btwn check code and transition either it be in compiler or interpreter.. it just doesn't go unhandled in Java code execution. 23-07-2020 Once pending_exception is detected in vm to Java transition we take control in exception path right(moving to the right handler or rethrowing by unwinding) here in JDK-8246727, we bypass exception handling and directly go to Java code with pending_exception set, it can be handled by some code(or cleared), but no guaranties about when and where. For the sake of convenience, assuming these floating exception are ok to be cleared. let me know if that is not right. 22-07-2020 I don't know what you mean by "floating pending exception". If you knew, for example, that an unsafe access error was due to code executed by the VM as part of the VM implementation, then you would want to "catch" it and process it without letting it propagate back to user code - but I doubt there are any such uses. (This is similar to getting a classloading/linkage error as part of compilation - you don't propagate it you just cancel the compilation and return to interpreted mode.) But generally no async exception should be ignored/cleared by the VM in any thread that executes application code. 22-07-2020 other question I have is, is it legal to have floating pending_exception in Java code? like the one we have seen in JDK-8246727, do we really want to honor those, or clearing/ignoring is fine? 21-07-2020 > I was pointing to https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/thread.cpp#L536 > where we set _pending_async_exception > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/thread.cpp#L2423 Just to clarify this part. While it takes a handshake/safepoint to set _pending_async_exception for the Thread::stop implementation, it may not be the case that the same handshake/safepoint results in the target thread calling handle_special_runtime_exit_condition(true) to actually start throwing the async exception that was just set as pending. > I missed this there is CHECK in the constructor. Good catch! That explains it. 21-07-2020 >> Just hitting a safepoint or a handshake is not sufficient I was pointing to https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/thread.cpp#L536 where we set _pending_async_exception https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/thread.cpp#L2423 //so perhaps there is something else between the end of the JavaCallWrapper constructor and the execution of the stub that stops us from reaching that assertion? Or perhaps there is something that will install and process the async-exception before we get to the point of executing the JavaCallWrapper, and we are just lucky with the timing// Actually i was checking for this in the code, but I couldn't find one. //JavaCallWrapper link(method, receiver, result, CHECK);// I missed this there is CHECK in the constructor. 21-07-2020 > so my question was wouldn't it be fairly common to get pending_exception set at java call method entry ? (i.e. there was some Safepoint and handshaking before java call and there was an async exception installed, It is actually relatively rare for the async exception check to happen. Just hitting a safepoint or a handshake is not sufficient. The primary time where an async exception is installed is when we will transition from being "in VM, or "in Native" to being "in Java" - e.g. returning to Java code from the VM ( the ThreadInVMFromJava destructor) - or equivalently when the VM calls into Java via the JavacallWrapper constructor. But I see your point about the assertion in the stub generator. If that method calling code is executed after the JavaCallWrapper has installed the async-exception then you should expect that assertion to fail. Note that the JavaCallWrapper clears any pending exception, except when it was an async exception, so perhaps there is something else between the end of the JavaCallWrapper constructor and the execution of the stub that stops us from reaching that assertion? Or perhaps there is something that will install and process the async-exception before we get to the point of executing the JavaCallWrapper, and we are just lucky with the timing. 20-07-2020 Thank you for the call stack. 20-07-2020 JavaCallWrapper[1] here check for as_special_runtime_exit_condition() and set pending_exception https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/javaCalls.cpp#L75 there is further assert in java call path here https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/stubGenerator_x86_64.cpp#L303 so my question was wouldn't it be fairly common to get pending_exception set at java call method entry ? (i.e. there was some Safepoint and handshaking before java call and there was an async exception installed, which got checked in java call wrapper here https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/javaCalls.cpp#L75) 20-07-2020 > can I get more frames for this stack-trace? callers for compile_method ? V [libjvm.so+0x915ca2] CompileBroker::compile_method(methodHandle const&, int, int, methodHandle const&, int, CompileTask::CompileReason, Thread*)+0x82 V [libjvm.so+0x16f878b] TieredThresholdPolicy::compile(methodHandle const&, int, CompLevel, JavaThread*)+0x18b V [libjvm.so+0x16fa85b] TieredThresholdPolicy::method_invocation_event(methodHandle const&, methodHandle const&, CompLevel, CompiledMethod*, JavaThread*)+0x1cb V [libjvm.so+0x16fbaf3] TieredThresholdPolicy::event(methodHandle const&, methodHandle const&, int, int, CompLevel, CompiledMethod*, JavaThread*)+0x253 V [libjvm.so+0x72afe0] Runtime1::counter_overflow(JavaThread*, int, Method*)+0x620 v ~RuntimeStub::counter_overflow Runtime1 stub J 1457 c1 com.sun.tools.javac.code.Types$15.visitClassType(Lcom/sun/tools/javac/code/Type$ClassType;Ljava/lang/Void;)Lcom/sun/tools/javac/code/Type; jdk.compiler@16-internal (130 bytes) @ 0x00007f85b56c72df [0x00007f85b56c6020+0x00000000000012bf] J 2208 c2 com.sun.tools.javac.code.Types$15.visitClassType(Lcom/sun/tools/javac/code/Type$ClassType;Ljava/lang/Object;)Ljava/lang/Object; jdk.compiler@16-internal (10 bytes) @ 0x00007f85bc8a8678 [0x00007f85bc8a8640+0x0000000000000038] J 1760 c2 com.sun.tools.javac.code.Type$ClassType.accept(Lcom/sun/tools/javac/code/Type$Visitor;Ljava/lang/Object;)Ljava/lang/Object; jdk.compiler@16-internal (9 bytes) @ 0x00007f85bc885e64 [0x00007f85bc885e20+0x0000000000000044] J 1587 c1 com.sun.tools.javac.code.Types.supertype(Lcom/sun/tools/javac/code/Type;)Lcom/sun/tools/javac/code/Type; jdk.compiler@16-internal (12 bytes) @ 0x00007f85b57490a4 [0x00007f85b5748f20+0x0000000000000184] j com.sun.tools.javac.comp.Lower$1.checkConflicts(Lcom/sun/tools/javac/util/JCDiagnostic$DiagnosticPosition;Lcom/sun/tools/javac/code/Symbol;Lcom/sun/tools/javac/code/Symbol$TypeSymbol;)V+207 jdk.compiler@16-internal j com.sun.tools.javac.comp.Lower$1.visitMethodDef(Lcom/sun/tools/javac/tree/JCTree$JCMethodDecl;)V+13 jdk.compiler@16-internal J 1414 c1 com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(Lcom/sun/tools/javac/tree/JCTree$Visitor;)V jdk.compiler@16-internal (6 bytes) @ 0x00007f85b569b5ec [0x00007f85b569b4e0+0x000000000000010c] J 1572 c2 com.sun.tools.javac.tree.TreeScanner.scan(Lcom/sun/tools/javac/tree/JCTree;)V jdk.compiler@16-internal (10 bytes) @ 0x00007f85bc85dc6c [0x00007f85bc85dc20+0x000000000000004c] J 2210 c2 com.sun.tools.javac.tree.TreeScanner.scan(Lcom/sun/tools/javac/util/List;)V jdk.compiler@16-internal (33 bytes) @ 0x00007f85bc8a4be4 [0x00007f85bc8a4b60+0x0000000000000084] j com.sun.tools.javac.tree.TreeScanner.visitClassDef(Lcom/sun/tools/javac/tree/JCTree$JCClassDecl;)V+45 jdk.compiler@16-internal j com.sun.tools.javac.comp.Lower$1.visitClassDef(Lcom/sun/tools/javac/tree/JCTree$JCClassDecl;)V+15 jdk.compiler@16-internal j com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(Lcom/sun/tools/javac/tree/JCTree$Visitor;)V+2 jdk.compiler@16-internal j com.sun.tools.javac.comp.Lower.checkConflicts(Lcom/sun/tools/javac/util/List;)V+29 jdk.compiler@16-internal j com.sun.tools.javac.comp.Lower.translateTopLevelClass(Lcom/sun/tools/javac/comp/Env;Lcom/sun/tools/javac/tree/JCTree;Lcom/sun/tools/javac/tree/TreeMaker;)Lcom/sun/tools/javac/util/List;+291 jdk.compiler@16-internal j com.sun.tools.javac.main.JavaCompiler.desugar(Lcom/sun/tools/javac/comp/Env;Ljava/util/Queue;)V+719 jdk.compiler@16-internal j com.sun.tools.javac.main.JavaCompiler.desugar(Ljava/util/Queue;)Ljava/util/Queue;+39 jdk.compiler@16-internal j com.sun.tools.javac.main.JavaCompiler.compile(Ljava/util/Collection;Ljava/util/Collection;Ljava/lang/Iterable;Ljava/util/Collection;)V+542 jdk.compiler@16-internal j com.sun.tools.javac.main.Main.compile([Ljava/lang/String;Lcom/sun/tools/javac/util/Context;)Lcom/sun/tools/javac/main/Main$Result;+615 jdk.compiler@16-internal j com.sun.tools.javac.main.Main.compile([Ljava/lang/String;)Lcom/sun/tools/javac/main/Main$Result;+15 jdk.compiler@16-internal j com.sun.tools.javac.Main.compile([Ljava/lang/String;)I+12 jdk.compiler@16-internal This was a javac run started by jtreg to compile the ExtraProperties file. 20-07-2020 > but it can install the exception right, (i.e.not checking but just setting pending_async_excption) Sorry the terminology is confusing. When there needs to be an async exception raised we will set a marker that there is a pending async exception, using either: thread->set_pending_async_exception(exception) or thread->set_pending_unsafe_access_error(); Those actions do not actually initiate the throwing of the exception, and there is no "pending exception" as reported by HAS_PENDING_EXCEPTION. The check for such a pending async exception happens if the thread calls handle_special_runtime_exit_condition(true). This check is fairly rare and it is this check that I think does not happen as part of a thread blocking (and unblocking) in the VM. The check can happen as part of the safepoint blocking mechanism, but it depends on what state the thread was in at the time of the safepoint. When we have one of these "pending async exception" markers, and the thread calls handle_special_runtime_exit_condition(true), then the async exception gets installed as the current pending exception, which means HAS_PENDING_EXCEPTION will return true and the exception is now in the process of being thrown and will propagate up the call stack once we hit code that checks for and responds to pending exceptions. 20-07-2020 //So far simply blocking in the VM does not appear to lead to an async exception check// but it can install the exception right, (i.e.not checking but just setting pending_async_excption) and the Java call after it will check for pending_async_exception and set pending_exception [1] and call_stub later (in debug mode) would assert on pending_exception. can't this happen ? //I observed a problem on this path just running javac.// can I get more frames for this stack-trace? callers for compile_method ? (I will try myself, as you said it is easily reproducible) [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/javaCalls.cpp#L75 20-07-2020 > I believe this should be in Xcomp/boot strapping mode) ? I observed a problem on this path just running javac. V [libjvm.so+0x16cf391] Thread::check_possible_async_exception_check()+0x31 V [libjvm.so+0xcfe20c] JavaCallWrapper::JavaCallWrapper(methodHandle const&, Handle, JavaValue*, Thread*)+0x1fc V [libjvm.so+0xd000dc] JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*, Thread*)+0x22c V [libjvm.so+0xd00de5] JavaCalls::call_virtual(JavaValue*, Klass*, Symbol*, Symbol*, JavaCallArguments*, Thread*)+0x4b5 V [libjvm.so+0xd01400] JavaCalls::call_virtual(JavaValue*, Handle, Klass*, Symbol*, Symbol*, Handle, Thread*)+0xf0 V [libjvm.so+0x1673efa] SystemDictionary::load_instance_class(Symbol*, Handle, Thread*)+0x18a V [libjvm.so+0x16727fa] SystemDictionary::resolve_instance_class_or_null(Symbol*, Handle, Handle, Thread*)+0x99a V [libjvm.so+0x1672b45] SystemDictionary::resolve_instance_class_or_null_helper(Symbol*, Handle, Handle, Thread*)+0x45 V [libjvm.so+0x1575071] SignatureStream::as_klass(Handle, Handle, SignatureStream::FailureMode, Thread*)+0x81 V [libjvm.so+0x12d8d2d] Method::load_signature_classes(methodHandle const&, Thread*)+0xcd V [libjvm.so+0x915725] CompileBroker::compile_method(methodHandle const&, int, int, methodHandle const&, int, CompileTask::CompileReason, DirectiveSet*, Thread*)+0x9f5 > Isn't this means async exception(Thread.Stop kind) can be installed at any safepoint and can reach call_stub pending_exception set (due to pending_async_exception check in JavaCallWrapper) A Thread.stop async exception is installed via a safepoint (in the future probably via a direct handshake). When that thread will actually install the async exception depends on how it got to the safepoint. So far simply blocking in the VM does not appear to lead to an async exception check. But yes JavaCallWrapper can lead to this check. > call_stub has assert for pending_exception and should fail in debug mode There are two steps to throwing an async exception. The first step makes the async-exception pending. This is not to be confused with the notion of a "pending exception" (which is an exception we have started to throw). When we hit the async exception check from handle_special_runtime_exit_condition, that pending async exception gets "installed", making it the actual "pending exception" which indicates it is now being thrown. So the assertion you refer to only catches this second step. 20-07-2020 //Obviously we are loading classes here and executing potentially arbitrary Java code (via the ClassLoader) and so exceptions, including async-exceptions are possible. Any general exceptions due to classloading should be ignored here, but it would be wrong to clear async exceptions.// I believe this should be in Xcomp/boot strapping mode) ? quoting comment form JDK-8246381 //The proposed fix is fine as far as it goes. I'm just saying there may be more to examine here for those other issues. The build_interpreter_method_data code takes a lock which means the thread can block, which means it can safepoint/handshake, which _might_ mean it can install an async exception. I'm trying to check on the details of that, but identifying the paths where we can potentially install async exceptions is non-trivial to work out.// Isn't this means async exception(Thread.Stop kind) can be installed at any safepoint and can reach call_stub pending_exception set (due to pending_async_exception check in JavaCallWrapper) call_stub has assert for pending_exception and should fail in debug mode https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/stubGenerator_x86_64.cpp#L303 20-07-2020 I did some experiments with my NoAsyncExceptionCheckVerifier (JDK-8249585) at these CHECK_AND_CLEAR sites: CompileBroker::compile_method if (comp->is_c2()) { method->constants()->resolve_string_constants(CHECK_AND_CLEAR_NULL); // Resolve all classes seen in the signature of the method // we are compiling. Method::load_signature_classes(method, CHECK_AND_CLEAR_NULL); } Obviously we are loading classes here and executing potentially arbitrary Java code (via the ClassLoader) and so exceptions, including async-exceptions are possible. Any general exceptions due to classloading should be ignored here, but it would be wrong to clear async exceptions. TieredThresholdPolicy::create_mdo if (mh->method_data() == NULL) { Method::build_interpreter_method_data(mh, CHECK_AND_CLEAR); } Initial testing did not show async exception checks happening on this path. ciReplay::initialize } else { EXCEPTION_CONTEXT; // m->_instructions_size = rec->_instructions_size; m->_instructions_size = -1; m->_interpreter_invocation_count = rec->_interpreter_invocation_count; m->_interpreter_throwout_count = rec->_interpreter_throwout_count; MethodCounters* mcs = method->get_method_counters(CHECK_AND_CLEAR); Initial testing did not show async exception checks happening on this path. To clarify: the paths shown may not have been executed by the tests run, and of course it may only be specific subpaths through the code that lead to the async exception check scenarios. 20-07-2020 To be clear, any use of an unconditional CLEAR_PENDING_EXCEPTION needs to be examined to see whether it could be an async exception that is pending. 15-07-2020