Produce dumps earlier for some types of OutOfMemoryErrors

[kgibm]

Reduce the timing window of system dumps not capturing critical stack frame locals by requesting OutOfMemoryError dump agents shortly after a subset of OOME conditions are detected. Such paths will also go through a lock to increase the chances that the thread that got the initial allocation failure is the one producing the dumps.

Add a new undocumented option -XX:-earlydumps to use the old behavior.

Closes: #23984

[keithc-ca]

Did you explore using a new "event flag" bit instead of a new field/parameter?

[kgibm]

Good idea. That will simplify the patch significantly.

[keithc-ca]

This will ignore all future system dumps. This seems problematic; agents are otherwise stateless.

[kgibm]

This seems problematic; agents are otherwise stateless

This is true but it's a very bounded and limited state machine because we know the exception will be thrown and handled soon after handleOutOfMemoryError runs so the state is only for a short window. I couldn't think of another approach that avoids running all the dump agents two times for each OOME.

Ideally, we could creates hooks anywhere that an OOME is thrown or about to be thrown, add this new handleOutOfMemoryError call there and then ignore OOMEs in rasDumpHookExceptionSysthrow and rasDumpHookExceptionThrow but I'm guessing that would be much more complicated, especially handling OOMEs thrown from Java code.

So I'm open to ideas, but I couldn't think of an alternative.

[keithc-ca]

I think the issue may go away if we have two events:

#define J9RAS_DUMP_ON_EXCEPTION_SYSTHROW  0x40000 /* existing event */
#define J9RAS_DUMP_ON_EXCEPTION_SYSTHROW _EARLY 0x8000000U /* new event */

Based on the command-line option, agents are configured to respond to one or the other (but not both).

[kgibm]

I might be missing something but the issue I see with that is rasDumpHookExceptionSysthrow or rasDumpHookExceptionThrow will still be handling the OOME, so no matter how we cause the agents to run in handleOutOfMemoryError, unless we somehow signal to rasDumpHookExceptionSysthrow or rasDumpHookExceptionThrow to not process the exception if it has already been processed in handleOutOfMemoryError, then each OOME agent will run twice for a single OOME. The only way I can think to make this work would be finding all potential causes of an OOME, calling handleOutOfMemoryError there and then skipping the OOME exception in rasDumpHookExceptionSysthrow or rasDumpHookExceptionThrow. This would be nice as it would more completely resolve the original issue, but that seems like a very complicated (especially OOMEs thrown from Java code as that will presumably have to hook deep into exception throwing and maybe JIT) and also brittle change (if someone ever adds a new path to creating an OOME).

[keithc-ca]

I don't think we need to make any adjustments for throw events, only for systhrow. Only the latter can be (initially) triggered by GC.

[kgibm]

Ahh, I see, but then we would need to ensure that every initial trigger of a systhrow for OutOfMemoryError calls handleOutOfMemoryError (and then we can ignore OutOfMemoryError exceptions in just rasDumpHookExceptionSysthrow). Are these two mgcalloc.cpp initial triggers the only ones? I'll explore more next week. It looks like systhrow exceptions only come through internalSetCurrentExceptionWithCause when currentThread->currentException == NULL.

[kgibm]

So if I'm understanding the flow correctly, there are various callers to internalSetCurrentExceptionWithCause with java/lang/OutOfMemoryError such as setHeapOutOfMemoryError, setNativeOutOfMemoryError, setThreadForkOutOfMemoryError, and setNativeBindOutOfMemoryError.

For example, a simple Java OOME might come from VM_BytecodeInterpreterFull::run calling allocateObject (which calls J9AllocateObject), receiving NULL, and then processing THROW_HEAP_OOM which calls setHeapOutOfMemoryError.

So I think we'd want to focus just on setHeapOutOfMemoryError. A solution might be to add some sort of "early" bitflag in exceptionNumber for the call to internalSetCurrentExceptionWithCause by setHeapOutOfMemoryError, and then in internalSetCurrentExceptionWithCause, only call TRIGGER_J9HOOK_VM_EXCEPTION_SYSTHROW if that flag is not set.

But then that would require that all triggers of setHeapOutOfMemoryError call the new handleOutOfMemoryError function. I went through all 33 files that call setHeapOutOfMemoryError, and while most are reacting to NULL triggers from J9AllocateObject or J9AllocateIndexableObject, there are some other paths such as runtime/j9vm/javanextvmi.cpp, runtime/j9vm/valhallavmi.cpp, runtime/vm/resolvesupport.cpp, and others that might not be as they're calling things like j9gc_objaccess_asConstantPoolObject, j9gc_createJavaLangString, checking some reflection field ID is null, etc., and I'm not sure if those all drive down to J9AllocateObject or J9AllocateIndexableObject.

So I'm not sure if I'm fully understanding the idea:

1. Are you thinking to just follow each one of those paths into setHeapOutOfMemoryError and add calls for their triggers to handleOutOfMemoryError and then skip TRIGGER_J9HOOK_VM_EXCEPTION_SYSTHROW for calls from setHeapOutOfMemoryError?
2. Also, this would mean that TRIGGER_J9HOOK_VM_EXCEPTION_SYSTHROW wouldn't be called for a subset of OOMEs and I'm not sure if there are any implications to that, even if the agents themselves still run?
3. I couldn't see how the new J9RAS_DUMP_ON_EXCEPTION_SYSTHROW_EARLY could play into this because even if we could ensure that all triggers of setHeapOutOfMemoryError themselves call handleOutOfMemoryError, then couldn't we just continue to use J9RAS_DUMP_ON_EXCEPTION_SYSTHROW from there?

[keithc-ca]

I was thinking about how to simplify this. The code in trigger.c could register the hook for J9HOOK_MM_PRIVATE_OUT_OF_MEMORY (unless early dumps are disabled) eliminating the need for modifying mgcalloc.cpp. That doesn't immediately solve the issue of state to avoid a second dump on J9RAS_DUMP_ON_EXCEPTION_SYSTHROW; but perhaps a reasonable place to put that is in the J9VMThread object - a flag would be set by the first hook callback (if it occurs) and the second hook, J9HOOK_VM_EXCEPTION_SYSTHROW would only clear the flag if set and otherwise ignore that callback. We don't need to add J9RAS_DUMP_ON_EXCEPTION_SYSTHROW_EARLY.

A remaining problem is the code in trigger.c that considers filter=java/lang/OutOfMemoryError will have problems with the early hook because no exception reference is (yet) available to handle # in the filter pattern. I couldn't think of a realistic use in the case of OOM, so perhaps it's a non-issue.

[keithc-ca]

Moving to draft state based on #23984 (comment).