RFR: 8347833: CrashOnOutOfMemory should stop GC threads before HeapDumpOnOutOfMemoryError

[David Holmes]

On Fri, 7 Feb 2025 18:07:32 GMT, Fairoz Matte <fmatte at openjdk.org> wrote:

> When CrashOnOutOfMemory  and HeapDumpOnOutOfMemoryError invoked together, we should make sure, it is performed in a single safepoint, this will avoid allowing other threads to run and throw OOM errors after the initial one is already under error logging.

The code needs adjusting to do what was intended.

Thanks

src/hotspot/share/utilities/vmError.cpp line 1952:

> 1950:   if(dumpHeap) {
> 1951:     HeapDumper::dump_heap_from_oome();
> 1952:   }

To be done at the same safepoint this code needs to be in `VM_ReportJavaOutOfMemory::doit()` - which is why the `dumpHeap` was to be passed to the `VM_ReportJavaOutOfMemory` constructor and stored in a field for `doit`.

test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryAndCrashOnOutOfMemory.java line 27:

> 25:  * @test TestHeapDumpOnOutOfMemoryAndCrashOnOutOfMemory
> 26:  * @summary Test verifies call to -XX:HeapDumpOnOutOfMemoryError and
> 27:  *          CrashOnOutOfMemoryError handled in a single safepoint operation

I can't see how you can possibly test this other than by having safepoint logging and checking the log output.

test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryAndCrashOnOutOfMemory.java line 43:

> 41:             try {
> 42:                 Object[] oa = new Object[Integer.MAX_VALUE];
> 43:                 for(int i = 0; i < oa.length; i++) {

Suggestion:

                for (int i = 0; i < oa.length; i++) {

test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryAndCrashOnOutOfMemory.java line 44:

> 42:                 Object[] oa = new Object[Integer.MAX_VALUE];
> 43:                 for(int i = 0; i < oa.length; i++) {
> 44:                     oa[i] = new Object[Integer.MAX_VALUE];

This will throw the "VM limit reached" OOME - does that trigger the heapdump etc processing?

test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryAndCrashOnOutOfMemory.java line 57:

> 55:         OutputAnalyzer output = new OutputAnalyzer(pb.start());
> 56:         int exitValue = output.getExitValue();
> 57:         if(0 != exitValue) {

Suggestion:

        if (exitValue != 0) {

-------------

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23519#pullrequestreview-2607628017
PR Review Comment: https://git.openjdk.org/jdk/pull/23519#discussion_r1950196937
PR Review Comment: https://git.openjdk.org/jdk/pull/23519#discussion_r1950197529
PR Review Comment: https://git.openjdk.org/jdk/pull/23519#discussion_r1950198533
PR Review Comment: https://git.openjdk.org/jdk/pull/23519#discussion_r1950204574
PR Review Comment: https://git.openjdk.org/jdk/pull/23519#discussion_r1950197996