-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8335553: [Graal] Compiler thread calls into jdk.internal.vm.VMSupport.decodeAndThrowThrowable and crashes in OOM situation #20083
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,11 +111,9 @@ bool Exceptions::special_exception(JavaThread* thread, const char* file, int lin | |
| } | ||
| #endif // ASSERT | ||
|
|
||
| if (!thread->can_call_java()) { | ||
| if (h_exception.is_null() && !thread->can_call_java()) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no reason to replace an existing exception object with a dummy exception object in the case where the current thread cannot call into Java. Since the exception object already exists, no Java call is necessary. This change is necessary to allow the libgraal exception translation mechanism to know that an OOME is being translated. |
||
| ResourceMark rm(thread); | ||
| const char* exc_value = h_exception.not_null() ? h_exception->print_value_string() : | ||
| h_name != nullptr ? h_name->as_C_string() : | ||
| "null"; | ||
| const char* exc_value = h_name != nullptr ? h_name->as_C_string() : "null"; | ||
| log_info(exceptions)("Thread cannot call Java so instead of throwing exception <%s%s%s> (" PTR_FORMAT ") \n" | ||
| "at [%s, line %d]\nfor thread " PTR_FORMAT ",\n" | ||
| "throwing pre-allocated exception: %s", | ||
|
|
@@ -205,7 +203,7 @@ void Exceptions::_throw_msg_cause(JavaThread* thread, const char* file, int line | |
| void Exceptions::_throw_cause(JavaThread* thread, const char* file, int line, Symbol* name, Handle h_cause, | ||
| Handle h_loader, Handle h_protection_domain) { | ||
| // Check for special boot-strapping/compiler-thread handling | ||
| if (special_exception(thread, file, line, h_cause)) return; | ||
dougxc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (special_exception(thread, file, line, Handle(), name)) return; | ||
| // Create and throw exception | ||
| Handle h_exception = new_exception(thread, name, h_cause, h_loader, h_protection_domain); | ||
| _throw(thread, file, line, h_exception, nullptr); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ | |
| import org.testng.annotations.Test; | ||
|
|
||
| import jdk.internal.misc.Unsafe; | ||
| import jdk.internal.vm.TranslatedException; | ||
| import jdk.internal.vm.VMSupport; | ||
|
|
||
| public class TestTranslatedException { | ||
|
|
@@ -100,6 +101,15 @@ public void encodeDecodeTest() throws Exception { | |
| try { | ||
| VMSupport.decodeAndThrowThrowable(4, 0L, true, false); | ||
| throw new AssertionError("expected decodeAndThrowThrowable to throw an exception"); | ||
| } catch (TranslatedException decoded) { | ||
| Assert.assertEquals(decoded.getCause().getClass(), OutOfMemoryError.class); | ||
| } catch (Throwable decoded) { | ||
| throw new AssertionError("unexpected exception: " + decoded); | ||
| } | ||
|
|
||
| try { | ||
| VMSupport.decodeAndThrowThrowable(5, 0L, true, false); | ||
| throw new AssertionError("expected decodeAndThrowThrowable to throw an exception"); | ||
| } catch (InternalError decoded) { | ||
| // Expected | ||
| } catch (Throwable decoded) { | ||
|
|
@@ -142,7 +152,7 @@ private void encodeDecode(Throwable throwable) throws Exception { | |
| VMSupport.decodeAndThrowThrowable(format, buffer, true, false); | ||
| throw new AssertionError("expected decodeAndThrowThrowable to throw an exception"); | ||
| } catch (Throwable decoded) { | ||
| assertThrowableEquals(throwable, decoded); | ||
| assertThrowableEquals(throwable, decoded.getCause()); | ||
| } | ||
| return; | ||
| } | ||
|
|
@@ -152,13 +162,15 @@ private void encodeDecode(Throwable throwable) throws Exception { | |
| } | ||
| } | ||
|
|
||
| private static void assertThrowableEquals(Throwable original, Throwable decoded) { | ||
| private static void assertThrowableEquals(Throwable originalIn, Throwable decodedIn) { | ||
| Throwable original = originalIn; | ||
| Throwable decoded = decodedIn; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of this renaming? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So that the printing down the bottom of this message shows the complete throwable, not just the cause on which the comparison failed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! I missed the reassign in the folded unchanged code. |
||
| try { | ||
| Assert.assertEquals(original == null, decoded == null); | ||
| while (original != null) { | ||
| if (Untranslatable.class.equals(original.getClass())) { | ||
| Assert.assertEquals(decoded.getClass().getName(), "jdk.internal.vm.TranslatedException"); | ||
| Assert.assertEquals(decoded.toString(), "jdk.internal.vm.TranslatedException[jdk.internal.vm.test.TestTranslatedException$Untranslatable]: test exception"); | ||
| Assert.assertEquals(decoded.getClass().getName(), "java.lang.InternalError"); | ||
| Assert.assertEquals(decoded.toString(), "java.lang.InternalError: test exception [jdk.internal.vm.test.TestTranslatedException$Untranslatable]"); | ||
| Assert.assertEquals(original.getMessage(), "test exception"); | ||
| } else { | ||
| Assert.assertEquals(decoded.getClass().getName(), original.getClass().getName()); | ||
|
|
@@ -182,10 +194,10 @@ private static void assertThrowableEquals(Throwable original, Throwable decoded) | |
| } | ||
| } catch (AssertionError e) { | ||
| System.err.println("original:["); | ||
| original.printStackTrace(System.err); | ||
| originalIn.printStackTrace(System.err); | ||
| System.err.println("]"); | ||
| System.err.println("decoded:["); | ||
| original.printStackTrace(System.err); | ||
| decodedIn.printStackTrace(System.err); | ||
| System.err.println("]"); | ||
| throw e; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we check for pending exception and break here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
CHECK_NULLmacro effectively does that.