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

[Kim Barrett]

> On May 30, 2019, at 11:31 AM, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
> 
> Hi David,
> 
> On 5/29/19 11:05 PM, David Holmes wrote:
>> Because we should not be changing the behaviour of os::die() because of what one very specific test that intends to end up calling die() desires.
> 
> Hmmmm... I'm proposing to change the behavior of os::die() because
> we have an option that says this:
> 
> src/hotspot/share/runtime/globals.hpp:
> 
>>   product(bool, CreateCoredumpOnCrash, true,                                \
>>           "Create core/mini dump on VM fatal error")                        \
> 
> so when we turn that option off, I'm asserting that we should not
> create a core dump on a VM fatal error. I happened to run into the
> issue with my work on JDK-8188872 and its specific test. The
> messenger in this case is JDK-8188872. Don't shoot the messenger.

[An overall, rather than point-by-point, response, so I'm not quoting
most of the message I'm replying to.]

I think David and I are viewing CreateCoredumpOnCrash as a flag for
controlling the behavior of os::abort. That's why David mentioned its
present usage by callers to that function. Although I see a number of
callers that aren't using it and either defaulting or explicitly
requesting a core dump, irrespective of that flag. There are also
places that os::abort unconditionally without a core dump that one
might wish could be asked for one (e.g. most, but not all, callers of
vm_abort).

But maybe those cases aren't "VM fatal errors"; maybe only code paths
that go through VMError count as such. That's the only way I can make
sense of the non-use of the flag by many callers of os::abort. I'm not
sure I like that viewpoint, but that seems to be what's going on.

I think TimeoutInErrorHandlingTest.java (the failing test from
JDK-8188872) is unusual, in that it is carefully directing the VM down
an ordinarily very weedy path that shouldn't be trodden even by some
problems in "normal" crashes.

I suspect most or all (though I only looked at a few) of the other
tests that specify -CreateCoredumpOnCrash are indeed attempting to
control the behavior of os::abort when called from VMError.  That
makes them different from TimeoutInErrorHandlingTest.

TimeoutInErrorHandlingTest makes use of a bespoke option to trigger
the desired behavior, -XX:+TestUnresponsiveErrorHandler. A completely
different way to achieve the effect you are trying for (having this
test not generate a core dump) would be to have the handling of that
option (either at argument processing time, or when VMError hits it)
use setrlimit to disable core dumps for the current process. Though we
don't appear to currently have os:: versions of the rlimit functions,
so there's a bit of work to be done there.

(Another test that might be similar is SecondaryErrorTest.java, but I
think it expects to just get a recursive error that ultimately still
ends in os::abort rather than os::die.)