Thank David for taking time. And sorry for misunderstand about the purpose of each VM options.
2016-03-15 9:38 GMT+09:00 David Holmes <david.hol...@oracle.com>: > Again clarify what you mean by this? Given CreateCoredumpOnCrash is usually > on this would produce warning messages in many execution contexts where core > dumps are disable (ie ulimit -c 0). That would be a bad thing. I aimed to notify warn before being faced with a crash if we run JVM on the execution context where core dumps are disabled (e.g. ulimit -c 0 by default) by something carelessness. I received many hs_err files which include the warm "Core dumps have been disabled." from users, so I want a notice to prevent it. Because hs_err includes useful information for after-the-fact analysis, but sometimes we need core image to reach the cause of crash, e.g., confirming the value of specified internal variables / structures. Thank you again for the detailed, I understood that my proposals are not reasonable. I have no good reasonable alternative ideas now, so withdraw this. Thanks, Yuji 2016-03-15 9:38 GMT+09:00 David Holmes <david.hol...@oracle.com>: > On 14/03/2016 6:16 PM, KUBOTA Yuji wrote: >> >> Hi David, >> >> Sorry, please let discussion a bit more. >> >> 2016-03-14 9:25 GMT+09:00 David Holmes <david.hol...@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. > > > What do you mean by "core dumping is disabled"? What is it you expect to be > checked? > > CreateCoredumpOnCrash (formerly CreateMinidumpOnCrash) was primarily a > windows flag to disable mini-dumps by default on Windows client builds. > Otherwise we expect the VM to always produce a core dump on crash. > >> However, the user who set CrashOnOutOfMemoryError need core images >> clearly, so I think jvm should return error if core dumping is >> disabled. > > > I disagree. The user who sets CrashOnOutOfMemoryError may also be setting > ShowMessageBoxOnError so they can attach a debugger or run other tools. > > The user who wants a core dump can enable core dumps explicitly if needed. > >> 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. > > > Again clarify what you mean by this? Given CreateCoredumpOnCrash is usually > on this would produce warning messages in many execution contexts where core > dumps are disable (ie ulimit -c 0). That would be a bad thing. > >> 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? > > > I don't agree that anything needs to be modified here - sorry. > > Thanks, > David > ----- > > > >> 2016-03-14 14:59 GMT+09:00 KUBOTA Yuji <kubota.y...@gmail.com>: >>> >>> Thanks David for looking and comments, >>> >>> 2016-03-14 9:25 GMT+09:00 David Holmes <david.hol...@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 >>>>> >>>> >
