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

[Daniel D. Daugherty]

Kim,

Thanks for the quick review! I see you sent two mesgs. I'm going to
reply to them separately to maintain context...


On 5/28/19 8:27 PM, Kim Barrett wrote:
>> On May 28, 2019, at 7:37 PM, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
>>
>> Resending with the bug's JBS link included...
>>
>> Greetings,
>>
>> While working on another bug (JDK-8188872), I happened to notice that
>> the test was generating core files even though the test is run with
>> the '-XX:+CreateCoredumpOnCrash' option.

Typo here: should be '-XX:-CreateCoredumpOnCrash'.


>>
>> After investigating, I discovered that the os::die() function does not
>> honor the CreateCoredumpOnCrash option which was a surprise to me.
>>
>> Webrev URL: http://cr.openjdk.java.net/~dcubed/8224793-webrev/0-for-jdk-jdk13/
>>
>> Bug: JDK-8224793 os::die() does not honor CreateCoredumpOnCrash option
>>       https://bugs.openjdk.java.net/browse/JDK-8224793
>>
>> Testing: Mach5 Tier[1-5]
>>           Included the fix in my latest round of 8153224 testing
>>           on Solaris-X64 where this bug reproduces quite a bit.
>>
>> Thanks, in advance, for any comments, suggestions, or questions.
>>
>> Dan
>>
>>
>
> [I assume in the description above you meant -CreateCoredumpOnCrash
> rather than +CreateCoredumpOnCrash.]

Yes, there is a typo above. Sorry for any confusion.


> 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."

Agreed. That's an error on my part. Here's the header comment:

src/hotspot/share/runtime/os.hpp:

   // Die immediately, no exit hook, no abort hook, no cleanup.
   static void die();


> 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.)

When I researched the man pages (macOS, Solaris and Ubuntu) for this bug,
I found that only the Solaris abort(3c) man page mentioned the possibility
of abort() generating a core file. It seems that the other platforms are
depending on other man pages to describe the result of SIGABRT. Sigh...


> 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.

I like it! I'll take a look at redoing that part with SIGKILL...


> These various implementations of os::die() look like they could be
> merged into os_posix.cpp.

I'd rather not do that with this bug. This was supposed to be simple
fix (ha!).

Thanks again for the review and on to your next reply...

Dan