-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8331208: Memory stress test that checks OutOfMemoryError stack trace fails #18925
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
a711f32
35e6b50
137ab23
d99fc56
977bdc2
545714a
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 |
|---|---|---|
|
|
@@ -118,4 +118,37 @@ class ClassAllocator: public MemAllocator { | |
| virtual oop initialize(HeapWord* mem) const; | ||
| }; | ||
|
|
||
| // Manages a scope where a failed heap allocation results in | ||
| // suppression of JVMTI "resource exhausted" events and | ||
| // throwing a shared, backtrace-less OOME instance. | ||
| // Used for OOMEs that will not be propagated to user code. | ||
| class InternalOOMEMark: public StackObj { | ||
| private: | ||
| bool _outer; | ||
| JavaThread* _thread; | ||
|
|
||
| public: | ||
| explicit InternalOOMEMark(JavaThread* thread) { | ||
| if (thread != nullptr) { | ||
| _outer = thread->is_in_internal_oome_mark(); | ||
| thread->set_is_in_internal_oome_mark(true); | ||
| _thread = thread; | ||
| } else { | ||
| _outer = false; | ||
| _thread = nullptr; | ||
| } | ||
|
Comment on lines
+136
to
+139
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. It isn't obvious to me how this part is intended to be used. I see it ties back to the retryable allocation "activate" mode, but I'm unclear what that means as well. 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. By "this part", do you mean the 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. "this part" means the "else branch" which means the null receiving constructor. Yeah that whole "null_on_fail" thing had me a bit perplexed and I see there is now a JBS issue filed. to kill it off as we always want null-on-fail. |
||
| } | ||
|
|
||
| ~InternalOOMEMark() { | ||
| if (_thread != nullptr) { | ||
| // Check that only InternalOOMEMark sets | ||
| // JavaThread::_is_in_internal_oome_mark | ||
| assert(_thread->is_in_internal_oome_mark(), "must be"); | ||
| _thread->set_is_in_internal_oome_mark(_outer); | ||
| } | ||
| } | ||
|
|
||
| JavaThread* thread() const { return _thread; } | ||
| }; | ||
|
|
||
| #endif // SHARE_GC_SHARED_MEMALLOCATOR_HPP | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
| #include "classfile/vmClasses.hpp" | ||
| #include "compiler/compileBroker.hpp" | ||
| #include "gc/shared/collectedHeap.hpp" | ||
| #include "gc/shared/memAllocator.hpp" | ||
| #include "gc/shared/oopStorage.inline.hpp" | ||
| #include "jvmci/jniAccessMark.inline.hpp" | ||
| #include "jvmci/jvmciCompilerToVM.hpp" | ||
|
|
@@ -92,39 +93,25 @@ static void deopt_caller() { | |
| } | ||
|
|
||
| // Manages a scope for a JVMCI runtime call that attempts a heap allocation. | ||
| // If there is a pending nonasync exception upon closing the scope and the runtime | ||
| // If there is a pending OutOfMemoryError upon closing the scope and the runtime | ||
| // call is of the variety where allocation failure returns null without an | ||
| // exception, the following action is taken: | ||
| // 1. The pending nonasync exception is cleared | ||
| // 1. The pending OutOfMemoryError is cleared | ||
| // 2. null is written to JavaThread::_vm_result | ||
| // 3. Checks that an OutOfMemoryError is Universe::out_of_memory_error_retry(). | ||
| class RetryableAllocationMark: public StackObj { | ||
| class RetryableAllocationMark { | ||
| private: | ||
| JavaThread* _thread; | ||
| InternalOOMEMark _iom; | ||
| public: | ||
| RetryableAllocationMark(JavaThread* thread, bool activate) { | ||
| if (activate) { | ||
| assert(!thread->in_retryable_allocation(), "retryable allocation scope is non-reentrant"); | ||
| _thread = thread; | ||
| _thread->set_in_retryable_allocation(true); | ||
| } else { | ||
| _thread = nullptr; | ||
| } | ||
| } | ||
| RetryableAllocationMark(JavaThread* thread, bool activate) : _iom(activate ? thread : nullptr) {} | ||
| ~RetryableAllocationMark() { | ||
| if (_thread != nullptr) { | ||
| _thread->set_in_retryable_allocation(false); | ||
| JavaThread* THREAD = _thread; // For exception macros. | ||
| JavaThread* THREAD = _iom.thread(); // For exception macros. | ||
| if (THREAD != nullptr) { | ||
| if (HAS_PENDING_EXCEPTION) { | ||
| oop ex = PENDING_EXCEPTION; | ||
| // Do not clear probable async exceptions. | ||
| CLEAR_PENDING_NONASYNC_EXCEPTION; | ||
| oop retry_oome = Universe::out_of_memory_error_retry(); | ||
| if (ex->is_a(retry_oome->klass()) && retry_oome != ex) { | ||
| ResourceMark rm; | ||
| fatal("Unexpected exception in scope of retryable allocation: " INTPTR_FORMAT " of type %s", p2i(ex), ex->klass()->external_name()); | ||
| THREAD->set_vm_result(nullptr); | ||
| if (ex->is_a(vmClasses::OutOfMemoryError_klass())) { | ||
| CLEAR_PENDING_EXCEPTION; | ||
| } | ||
|
Comment on lines
+112
to
114
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. Just an observation but the original code will clear all exceptions except for an "async" exception, which these days is only the InternalError thrown by unsafe-access-errors. But the new code will only clear OOME thus allowing the (as expected) InternalError to remain, but also any other VirtualMachineErrors that may have arisen e.g. StackOverflowError. I actually think this is more correct, but it does seem a change in behaviour that we may need to be wary of. 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. In the context of Graal, it doesn't really make much of a difference as the Graal stub that calls this runtime routine will clear all exceptions anyway. But yes, I think limiting the clearing here to OOME is better. |
||
| _thread->set_vm_result(nullptr); | ||
| } | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.