RFR(XXS): 8224793: os::die() does not honor CreateCoredumpOnCrash option

[David Holmes]

On 29/05/2019 11:03 am, Kim Barrett wrote:
>> On May 28, 2019, at 8:27 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>> Calling os::exit here seems wrong.  We may have already tried calling
>> os::exit and gotten here because of problems therein.  And the doc
>> comment for os::die says "no exit hook, no abort hook, no cleanup."

Yes calling ::exit is wrong.

>>
>> SIGABRT (signaled by ::abort) has a default action of Core, which is
>> the reason for the current behavior.  (And ::abort will invoke that
>> action if the installed action returns rather than terminating the
>> process.)  If no core is wanted, SIGKILL has a default action of
>> Terminate, and that behavior can't be replaced.  So issuing a SIGKILL
>> when -CreateCoredumpOnCrash seems like it should get the desired
>> behavior.
>>
>> These various implementations of os::die() look like they could be
>> merged into os_posix.cpp.
>>
> 
> And looking at this some more, I think I agree with David's comment in
> the CR, that os::die *should* always dump core.  It gets called when
> things have gone horribly wrong, and a core dump might be the only way
> to understand what went wrong.

Yes - to me "CreateCoredumpOnCrash" only applies to a "crash" that is 
the result of the VM encountering a (first level) fatal error. If we get 
secondary crashes that lead to die() then we're in unexpected territory 
and should dump core by default.

> Maybe there are paths to os::die that ought to be calling os::abort
> instead?

+1 - maybe something special to handle the crash handler test case?

> I think os::abort probably should not be calling ::exit either, but
> should be raising SIGKILL if no core dump is requested.  os::exit is
> the path to exit functions and the like.

Yes that is odd. I was going to say people may be relying on it calling 
atexit functions, but the fact it calls abort else exit depending on the 
need to dump core suggests no-one could be relying on that.

Thanks,
David
-----

> It's unfortunate that the Linux version of os::abort needs to do
> something special for DumpPrivateMappingsInCore; without that, we
> could have a merged posix version of os::abort.