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
