Check core dumping is enabled when set CrashOnOutOfMemoryError

[KUBOTA Yuji]

Hi David,

Sorry, please let discussion a bit more.

2016-03-14 9:25 GMT+09:00 David Holmes <david.holmes at oracle.com>:
> First I'm not convinced that you necessarily want to enable core dumps just
> because you want to crash on OOM.
>
> Second I think anyone setting CrashOnOutOfMemoryError can easily remember to
> set CreateCoredumpOnCrash if they want it.

I think most users do not pay attention about core dumping because
 * CreateCoredumpOnCrash is enabled by default (jdk9/hs-rt).
 * Core images not necessarily needed (except crash).
So we should warn if core dumping is disabled and
CreateCoredumpOnCrash is enabled, but continue to launch jvm.

However, the user who set CrashOnOutOfMemoryError need core images
clearly, so I think jvm should return error if core dumping is
disabled.
For example, I want to use CrashOnOOME instead of HeapDumpOnOOME,
because we can exit jvm definitely and get heapdump and other helpful
information by serviceability tools.

In conclusion, I have two proposals for avoiding the lack of information.
1. Launch jvm with warn message if CreateCoredumpOnCrash is enabled
but core dumping is disabled.
2. 1 + Quit with error message if CrashOnOutOfMemoryError is enabled
but core dumping is disabled.
I think I should implement 2 in arguments.cpp.

Could you share me your thought?

2016-03-14 14:59 GMT+09:00 KUBOTA Yuji <kubota.yuji at gmail.com>:
> Thanks David for looking and comments,
>
> 2016-03-14 9:25 GMT+09:00 David Holmes <david.holmes at oracle.com>:
>> First I'm not convinced that you necessarily want to enable core dumps just
>> because you want to crash on OOM.
>
> Yes, that's right. I just want to avoid the failed of core dumping
> when we want it clearly by setting options.
>
>> Second I think anyone setting CrashOnOutOfMemoryError can easily remember to
>> set CreateCoredumpOnCrash if they want it.
>>
>> Third if I was going to force this on then I would do it directly in
>> arguments.cpp, not by adding a constraint function.
>
> Thanks for sharing information! I will check the implementation of
> CreateCoredumpOnCrash and update my patch to use arguments.cpp.
>
> Thanks,
> Yuji
>
>>
>> Thanks,
>> David
>> -----
>>
>>
>>> So I create a patch to check the core dumping, please review it.
>>> I do not have an account of openjdk, so I need sponsor.
>>>
>>> I send to serviceability-dev because JDK-8138745 [1] has been
>>> reviewed here. If unfit, please let me know.
>>>
>>> [1]:Implement ExitOnOutOfMemory and CrashOnOutOfMemory in HotSpot
>>>       https://bugs.openjdk.java.net/browse/JDK-8138745
>>>
>>> diff --git a/src/share/vm/runtime/commandLineFlagConstraintsRuntime.cpp
>>> b/src/share/vm/runtime/commandLineFlagConstraintsRuntime.cpp
>>> --- a/src/share/vm/runtime/commandLineFlagConstraintsRuntime.cpp
>>> +++ b/src/share/vm/runtime/commandLineFlagConstraintsRuntime.cpp
>>> @@ -119,6 +119,17 @@
>>>     }
>>>   }
>>>
>>> +Flag::Error CrashOnOutOfMemoryErrorFunc(bool value, bool verbose) {
>>> +  char buffer[O_BUFLEN];
>>> +  os::check_dump_limit(buffer, sizeof(buffer));
>>> +  if (!VMError::coredump_status) {
>>> +    CommandLineError::print(verbose,
>>> +                            "%s\n", VMError::coredump_message);
>>> +    return Flag::VIOLATES_CONSTRAINT;
>>> +  }
>>> +  return Flag::SUCCESS;
>>> +}
>>> +
>>>   Flag::Error PerfDataSamplingIntervalFunc(intx value, bool verbose) {
>>>     if ((value % PeriodicTask::interval_gran != 0)) {
>>>       CommandLineError::print(verbose,
>>> diff --git a/src/share/vm/runtime/commandLineFlagConstraintsRuntime.hpp
>>> b/src/share/vm/runtime/commandLineFlagConstraintsRuntime.hpp
>>> --- a/src/share/vm/runtime/commandLineFlagConstraintsRuntime.hpp
>>> +++ b/src/share/vm/runtime/commandLineFlagConstraintsRuntime.hpp
>>> @@ -43,6 +43,8 @@
>>>   Flag::Error BiasedLockingBulkRevokeThresholdFunc(intx value, bool
>>> verbose);
>>>   Flag::Error BiasedLockingDecayTimeFunc(intx value, bool verbose);
>>>
>>> +Flag::Error CrashOnOutOfMemoryErrorFunc(bool value, bool verbose);
>>> +
>>>   Flag::Error PerfDataSamplingIntervalFunc(intx value, bool verbose);
>>>
>>>   #endif /* SHARE_VM_RUNTIME_COMMANDLINEFLAGCONSTRAINTSRUNTIME_HPP */
>>> diff --git a/src/share/vm/runtime/globals.hpp
>>> b/src/share/vm/runtime/globals.hpp
>>> --- a/src/share/vm/runtime/globals.hpp
>>> +++ b/src/share/vm/runtime/globals.hpp
>>> @@ -1401,6 +1401,7 @@
>>>     product(bool, CrashOnOutOfMemoryError, false,
>>> \
>>>             "JVM aborts, producing an error log and core/mini dump, on the
>>> "  \
>>>             "first occurrence of an out-of-memory error")
>>> \
>>> +          constraint(CrashOnOutOfMemoryErrorFunc,AfterErgo)
>>> \
>>>
>>> \
>>>     /* tracing */
>>> \
>>>
>>> \
>>> diff --git a/src/share/vm/utilities/vmError.hpp
>>> b/src/share/vm/utilities/vmError.hpp
>>> --- a/src/share/vm/utilities/vmError.hpp
>>> +++ b/src/share/vm/utilities/vmError.hpp
>>> @@ -65,14 +65,6 @@
>>>     // so use thread id instead of Thread* to identify thread.
>>>     static volatile intptr_t first_error_tid;
>>>
>>> -  // Core dump status, false if we have been unable to write a
>>> core/minidump for some reason
>>> -  static bool coredump_status;
>>> -
>>> -  // When coredump_status is set to true this will contain the
>>> name/path to the core/minidump,
>>> -  // if coredump_status if false, this will (hopefully) contain a
>>> useful error explaining why
>>> -  // no core/minidump has been written to disk
>>> -  static char coredump_message[O_BUFLEN];
>>> -
>>>
>>>     // set signal handlers on Solaris/Linux or the default exception
>>> filter
>>>     // on Windows, to handle recursive crashes.
>>> @@ -111,6 +103,14 @@
>>>     // Record status of core/minidump
>>>     static void record_coredump_status(const char* message, bool status);
>>>
>>> +  // Core dump status, false if we have been unable to write a
>>> core/minidump for some reason
>>> +  static bool coredump_status;
>>> +
>>> +  // When coredump_status is set to true this will contain the
>>> name/path to the core/minidump,
>>> +  // if coredump_status if false, this will (hopefully) contain a
>>> useful error explaining why
>>> +  // no core/minidump has been written to disk
>>> +  static char coredump_message[O_BUFLEN];
>>> +
>>>     // support for VM.info diagnostic command
>>>     static void print_vm_info(outputStream* st);
>>>
>>>
>>> Thanks,
>>> Yuji
>>>
>>