RFR: 8278125: Some preallocated OOMEs are missing stack trace [v2]

[David Holmes]

On 5/12/2021 4:53 am, Coleen Phillimore wrote:
> On Fri, 3 Dec 2021 05:51:48 GMT, Yi Yang <yyang at openjdk.org> wrote:
> 
>>> Our customer found that the PreallocatedOutOfMemoryErrorCount is 4 by default. It ought to pre-allocate 4 OOMEs with stack trace, but in fact, no matter how many OOMEs are thrown, only 2 OOMEs have proper stack trace. This is because PreallocatedOutOfMemoryErrorCount is consumed multiple times:
>>>
>>> VM tries to allocate a huge object while Java heap is insufficient to accommodate, it goes to:
>>>
>>> https://github.com/openjdk/jdk/blob/b79554bb5cef14590d427543a40efbcc60c66548/src/hotspot/share/gc/shared/memAllocator.cpp#L115-L136
>>>
>>> Line 135(`Universe::out_of_memory_error_java_heap`->`Universe::gen_out_of_memory_error`) and line 136(`THROW_OOP_`->`Exceptions::_throw`->`Exceptions::count_out_of_memory_exceptions`->`Universe::out_of_memory_error_java_heap`->`Universe::gen_out_of_memory_error`) call `Universe::gen_out_of_memory_error` twice in a single OOM event, which leads this unexpected scenario. Proposed fix is to remove such call in `Exceptions::count_out_of_memory_exceptions`.
>>>
>>> Thanks.
>>
>> Yi Yang has updated the pull request incrementally with one additional commit since the last revision:
>>
>>    Universe::is_out_of_memory_xx
> 
> Changes requested by coleenp (Reviewer).
> 
> src/hotspot/share/memory/universe.cpp line 597:
> 
>> 595:   const char * msg_str = java_lang_String::as_utf8_string(msg_oop);
>> 596:   return strcmp(msg, msg_str) == 0;
>> 597: }
> 
> Yes, I agree with David about strcmp.

I was only commenting on its placement, not its use.

> Could you add this instead (it was trying to compare the oops for equality, and unintentionally generated a new one).
> 
> oop Universe::is_out_of_memory_error_metaspace(oop exception) {
>    return out_of_memory_errors()->obj_at(_oom_metaspace)) == exception;
> }

This isn't correct. This will only match with the "default" exception 
object that gets created and which is only used after the pre-allocated 
exceptions (which can still have their stacktrace set) have been used 
up. So the count would be off by up to PreallocatedOutOfMemoryErrorCount.

David
-----

> src/hotspot/share/utilities/exceptions.cpp line 466:
> 
>> 464:   if (Universe::is_out_of_memory_error_metaspace(exception())) {
>> 465:      Atomic::inc(&_out_of_memory_error_metaspace_errors, memory_order_relaxed);
>> 466:   } else if (Universe::is_out_of_memory_error_class_metaspace(exception())) {
> 
> It was me.  I was confused by the function name and thought it just returned the oop.
> 
> test/hotspot/jtreg/runtime/ErrorHandling/GenOutOfMemoryError.java line 60:
> 
>> 58:         Asserts.assertTrue(4/*PreallocatedOutOfMemoryErrorCount is default to 4*/ == OOME_HAS_STACK_CNT, "Some OOMEs do not have stacktraces");
>> 59:     }
>> 60: }
> 
> missing newline apparently.
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/6672
>