RFR: JDK-8155004: CrashOnOutOfMemoryError doesn't work for OOM caused by inability to create threads

[David Holmes]

Hi Thomas,

On 21/04/2021 5:48 pm, Thomas Stuefe wrote:
> Hi David,
> 
> (snip)
> 
>>> p.s. I do not understand your quotes around "bug", nor your hesitance to fix this. I have seen quite a number of desperate setups customers have around crashy or OOMy JVMs, to analyse and to restart quickly. Anything in that area we can do helps.
>>
>> My hesitance is because the intent of these flags is to provide a
>> response for Java heap exhaustion, not to provide a response to every
>> place that java.lang.OutOfMemoryError may be thrown. You've picked this
>> one case of native thread exhaustion to add-in but that just makes it
>> inconsistent. Why should that case be handled and not all the others? It
>> is unfortunate that there is more than one way to read OutOfMemoryError,
>> but it should be read as out-of-Java-heap-memory-error. We have had bug
>> reports when someone uses this flag and their code does "throw new
>> OutOfMemoryError() and they want to know why the VM didn't crash!
>>
>> I wouldn't object to a general broadening of what this flag applies to
>> but I do object to cherry-picking a single case, resulting in the flag
>> have no well-defined meaning at all.
>>
> 
> Okay, I understand your concern now better.
> 
> If you do not object in principle to broadening of ..OnOutOfMemoryError to cover more cases, but just to the fact that this RFE only handles this one case of resource exhaustion, I think we have common ground. Both Yasumasa and me are fully in favour of making the ..OnOutOfMemoryError coverage as complete as possible. I would prefer to do this one RFE at a time, since that way we can put in work when we have free cycles, and these smaller patches are more easy to backport.

I've done some more research and I'm less convinced that this is worth 
pursuing. You might think it good to add more cases to when these flags 
get applied but what if existing users of these flags do not want them 
to be applied to these new cases? You might argue that being unable to 
start a native thread is something that should trigger a heapdump, or 
abort the VM, but somebody else may not want that. And even if we agree 
that most people might want this case, what about the other cases? I 
certainly don't think that any throwing of OutOfMemoryError should 
trigger these flags as many of them do not actually reflect being 
out-of-memory: the OOME is just a proxy for out-of-some-resource.

> Today ..OnOutOfMemoryError is invoked on OOM for both heap- and metaspace exhaustion, and AFAICS has been this way since at least jdk 11. So it is not consistently applied to java heap ooms only.

Okay you got me, but these are both allocation regions directly related 
to Java-level allocation: either instances or classes.

>> And per your latest email perhaps we need new generic flags for
>> resource-exhaustion.
> 
> I'd like that very much. But let me make an argument for fixing the ..OnOutOfMemoryError first, where fixing means making them apply to as many cases of thrown OOMs as possible:
> 
> The public does not know when these hooks fire and reasonably assume they fire for all instances of OOM, regardless the cause. To my knowledge we never told them differently. Since it is difficult enough to educate them about new options - look at AbortVMOnException, not many seem to know that one - I'd go with the flow and let the switches do what the public always thought they would do. There are a lot of StackOverFlow articles describing these switches, for instance.

An initial google search did not reveal to me any huge issues with these 
flags. And most of the time people want to get a heap dump because they 
only expect HeapDumpOnOutOfMemory to do a heap dump when out of heap memory.

I think if you want to offer similar options for other circumstances, 
then it is better to add new flags to deal with that, rather than 
takeover the existing ones and potentially change behaviour in a way 
that users have no control over.

Sorry.

Cheers,
David

> Cheers, Thomas
> 
>>
>> Cheers,
>> David
>> -----
>>
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/3586
>