Skip to content

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

8331208: Memory stress test that checks OutOfMemoryError stack trace fails #18925

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a711f32
generalized RetryableAllocationMark to SandboxedOOMEMark
dougxc Apr 23, 2024
35e6b50
only support disabling JVMTI events in SandboxedOOMEMark scope
dougxc Apr 25, 2024
137ab23
unconditionally disable JVMTI resource exhausted events in scope of a…
dougxc Apr 25, 2024
d99fc56
rename SandboxedOOMEMark to InternalOOMEMark
dougxc May 1, 2024
977bdc2
don't perform XX actions for OOME when in scope of an InternalOOMEMark
dougxc May 1, 2024
545714a
addressed review comments and suggestions
dougxc May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/compiler/compileBroker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1397,7 +1397,7 @@ nmethod* CompileBroker::compile_method(const methodHandle& method, int osr_bci,
assert(!HAS_PENDING_EXCEPTION, "No exception should be present");
// some prerequisites that are compiler specific
if (comp->is_c2() || comp->is_jvmci()) {
SandboxedOOMEMark som(THREAD);
InternalOOMEMark iom(THREAD);
method->constants()->resolve_string_constants(CHECK_AND_CLEAR_NONASYNC_NULL);
// Resolve all classes seen in the signature of the method
// we are compiling.
Expand Down
7 changes: 3 additions & 4 deletions src/hotspot/share/gc/shared/memAllocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,9 @@ bool MemAllocator::Allocation::check_out_of_memory() {
}

const char* message = _overhead_limit_exceeded ? "GC overhead limit exceeded" : "Java heap space";
// -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError support
report_java_out_of_memory(message);

if (!_thread->in_sandboxed_oome_mark()) {
if (!_thread->in_internal_oome_mark()) {
// -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError support
report_java_out_of_memory(message);
if (JvmtiExport::should_post_resource_exhausted()) {
JvmtiExport::post_resource_exhausted(
JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR | JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
Expand Down
18 changes: 9 additions & 9 deletions src/hotspot/share/gc/shared/memAllocator.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,29 +122,29 @@ class ClassAllocator: public MemAllocator {
// 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 SandboxedOOMEMark: public StackObj {
class InternalOOMEMark: public StackObj {
private:
bool _outer;
JavaThread* _thread;

public:
SandboxedOOMEMark(JavaThread* thread) {
InternalOOMEMark(JavaThread* thread) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: add a comment:

// Passing a null thread allows for a no-op implementation for contexts that will suppress
// throwing of the OOME - see RetryableAllocationMark.

I was wondering if we really need this. AFAICS it would be harmless to always pass in the current thread and set the thread's field because when we would have passed null then no exception would be thrown anyway. It seems the null thread is only used as a means for RAM to track whether activate was false. But I guess a no-op IOM achieves the same goal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing of the OOME is never suppressed by InternalOOMEMark. It only changes how the OOME is initialized.
When RetryableAllocationMark passes thread == null, it wants the normal OOME initialization to be done and JVMTI events to be fired.
In the context of https://bugs.openjdk.org/browse/JDK-8331429, I propose to leave this PR as is. That issue will remove activate altogether (cc @mur47x111 ).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
InternalOOMEMark(JavaThread* thread) {
explicit InternalOOMEMark(JavaThread* thread) {

if (thread != nullptr) {
_outer = thread->in_sandboxed_oome_mark();
thread->set_in_sandboxed_oome_mark(true);
_outer = thread->in_internal_oome_mark();
thread->set_in_internal_oome_mark(true);
_thread = thread;
} else {
_outer = false;
_thread = nullptr;
}
}

~SandboxedOOMEMark() {
~InternalOOMEMark() {
if (_thread != nullptr) {
// Check that only SandboxedOOMEMark sets
// JavaThread::_in_sandboxed_oome_mark
assert(_thread->in_sandboxed_oome_mark(), "must be");
_thread->set_in_sandboxed_oome_mark(_outer);
// Check that only InternalOOMEMark sets
// JavaThread::_in_internal_oome_mark
assert(_thread->in_internal_oome_mark(), "must be");
_thread->set_in_internal_oome_mark(_outer);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/jvmci/jvmciRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ static void deopt_caller() {
// 2. null is written to JavaThread::_vm_result
class RetryableAllocationMark {
private:
SandboxedOOMEMark _som;
InternalOOMEMark _iom;
public:
RetryableAllocationMark(JavaThread* thread, bool activate) : _som(activate ? thread : nullptr) {}
RetryableAllocationMark(JavaThread* thread, bool activate) : _iom(activate ? thread : nullptr) {}
~RetryableAllocationMark() {
JavaThread* THREAD = _som.thread();
JavaThread* THREAD = _iom.thread();
if (THREAD != nullptr) {
if (HAS_PENDING_EXCEPTION) {
oop ex = PENDING_EXCEPTION;
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/oops/klass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,8 +873,8 @@ void Klass::set_archived_java_mirror(int mirror_index) {

void Klass::check_array_allocation_length(int length, int max_length, TRAPS) {
if (length > max_length) {
report_java_out_of_memory("Requested array size exceeds VM limit");
if (!THREAD->in_sandboxed_oome_mark()) {
if (!THREAD->in_internal_oome_mark()) {
report_java_out_of_memory("Requested array size exceeds VM limit");
JvmtiExport::post_array_size_exhausted();
THROW_OOP(Universe::out_of_memory_error_array_size());
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/runtime/deoptimization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,7 @@ bool Deoptimization::realloc_objects(JavaThread* thread, frame* fr, RegisterMap*

InstanceKlass* ik = InstanceKlass::cast(k);
if (obj == nullptr && !cache_init_error) {
SandboxedOOMEMark som(THREAD);
InternalOOMEMark iom(THREAD);
#if COMPILER2_OR_JVMCI
if (EnableVectorSupport && VectorSupport::is_vector(ik)) {
obj = VectorSupport::allocate_vector(ik, fr, reg_map, sv, THREAD);
Expand All @@ -1266,11 +1266,11 @@ bool Deoptimization::realloc_objects(JavaThread* thread, frame* fr, RegisterMap*
TypeArrayKlass* ak = TypeArrayKlass::cast(k);
assert(sv->field_size() % type2size[ak->element_type()] == 0, "non-integral array length");
int len = sv->field_size() / type2size[ak->element_type()];
SandboxedOOMEMark som(THREAD);
InternalOOMEMark iom(THREAD);
obj = ak->allocate(len, THREAD);
} else if (k->is_objArray_klass()) {
ObjArrayKlass* ak = ObjArrayKlass::cast(k);
SandboxedOOMEMark som(THREAD);
InternalOOMEMark iom(THREAD);
obj = ak->allocate(sv->field_size(), THREAD);
}

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/javaThread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ JavaThread::JavaThread() :
#endif
#endif
_jni_attach_state(_not_attaching_via_jni),
_in_sandboxed_oome_mark(false),
_in_internal_oome_mark(false),
#if INCLUDE_JVMCI
_pending_deoptimization(-1),
_pending_monitorenter(false),
Expand Down
10 changes: 5 additions & 5 deletions src/hotspot/share/runtime/javaThread.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class ContinuationEntry;
class DeoptResourceMark;
class JNIHandleBlock;
class JVMCIRuntime;
class SandboxedOOMEMark;
class InternalOOMEMark;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to get this sorted as the other forward declarations.


class JvmtiDeferredUpdates;
class JvmtiSampledObjectAllocEventCollector;
Expand Down Expand Up @@ -335,8 +335,8 @@ class JavaThread: public Thread {
// of _attaching_via_jni and transitions to _attached_via_jni.
volatile JNIAttachStates _jni_attach_state;

// In scope of a SandboxedOOMEMark?
bool _in_sandboxed_oome_mark;
// In scope of an InternalOOMEMark?
bool _in_internal_oome_mark;

#if INCLUDE_JVMCI
// The _pending_* fields below are used to communicate extra information
Expand Down Expand Up @@ -713,8 +713,8 @@ class JavaThread: public Thread {
MemRegion deferred_card_mark() const { return _deferred_card_mark; }
void set_deferred_card_mark(MemRegion mr) { _deferred_card_mark = mr; }

bool in_sandboxed_oome_mark() const { return _in_sandboxed_oome_mark; }
void set_in_sandboxed_oome_mark(bool b) { _in_sandboxed_oome_mark = b; }
bool in_internal_oome_mark() const { return _in_internal_oome_mark; }
void set_in_internal_oome_mark(bool b) { _in_internal_oome_mark = b; }

#if INCLUDE_JVMCI
jlong pending_failed_speculation() const { return _pending_failed_speculation; }
Expand Down