On 8/12/2015 9:26 PM, cheleswer sahu wrote:
Hi David,
Please review the code changes in updated webrev
http://cr.openjdk.java.net/~kevinw/8138745/webrev.02/
Looks good! Only one more thing I forgot to mention - no new webrev
needed. When you use the testlibrary you also need to ensure it gets
built, so you need to add:
jdk.test.lib.*
to your @build lines.
Thanks,
David
Regards,
Cheleswer
On 12/8/2015 4:36 AM, David Holmes wrote:
Hi Cheleswer,
Thanks. Just a few minor updates ...
On 3/12/2015 4:58 PM, cheleswer sahu wrote:
Hi,
Thanks David and Staffan for your comments. Please review the code
changes in the updated webrev below
http://cr.openjdk.java.net/~kevinw/8138745/webrev.01/
src/share/vm/runtime/globals.hpp
As I said previously this terminology:
+ "JVM aborts and produces text and binary crash files on
the " \
should be consistent with what we already use. So I suggest:
+ "JVM aborts, producing an error log and core/mini dump, on
the " \
In the tests either "throw new Error(...)" or "throw new
RuntimeException(...)" but don't switch between them.
test/runtime/ErrorHandling/TestExitOnOutOfMemoryError.java
45 } catch (OutOfMemoryError err) {
46 throw err;
47 }
should be:
45 } catch (OutOfMemoryError err) {
46 throw new Error("OOME didn't terminate JVM!");
47 }
---
test/runtime/ErrorHandling/TestCrashOnOutOfMemoryError.java
50 } catch (OutOfMemoryError err) {
51 throw err;
52 }
should be:
50 } catch (OutOfMemoryError err) {
51 throw new Error("OOME didn't abort JVM!");
52 }
Here:
55 ProcessBuilder pb =
ProcessTools.createJavaProcessBuilder("-XX:+CrashOnOutOfMemoryError",
56 "-Xmx64m",
TestCrashOnOutOfMemoryError.class.getName(),"throwOOME");
you also need to disable core dump generation:
-XX:-CreateCoredumpOnCrash otherwise you may run into issues with
core dumps taking a long time to generate and the test times out.
64 /* Actual output should look like this:
"should look _something_ like this". The actual text will depend on
the OS and its core dump processing.
Thanks,
David
Regards,
Cheleswer
On 11/26/2015 11:58 AM, David Holmes wrote:
Sorry forgot the tests ....
test/runtime/ErrorHandling/TestExitOnOutOfMemoryError.java
This test is checking that new Object[Integer.MAX_VALUE]; caused the
"Requested array size exceeds VM limit" failure _but_ it doesn't
actually do anything to verify that the VM terminated because of the
ExitOnOutOfMemory flag. I suggest:
a) augment the termination message in the VM as I suggested earlier so
that you can be sure you hit the termination code
b) check for a zero/non-zero return code as appropriate
c) Add: throw new Error("OOME was not triggered"); after line 41.
d) Put a try/catch(OOME) around the allocation and throw an Error if
you get to the catch block
That way we will get a test failure when Arrays 2.0 allows such
massive arrays to be created :)
Similar considerations for TestCrashOnOutOfMemoryError.java, but you
also need to disable core dump generation.
David
-----
On 26/11/2015 3:43 PM, David Holmes wrote:
Hi,
On 25/11/2015 10:40 PM, cheleswer sahu wrote:
Hi,
Please review the code changes for
"https://bugs.openjdk.java.net/browse/JDK-8138745";.
Web review link:
<http://cr.openjdk.java.net/%7Ekevinw/8138745/webrev.00/>http://cr.openjdk.java.net/~kevinw/8138745/webrev.00/
Enhancement Brief:
ExitOnOutOfMemoryError: When user enable this option, the JVM
exits on
the first occurrence of an out-of-memory error. It can be used if
user
prefer restarting an instance of the JVM rather than handling out of
memory errors.
CrashOnOutOfMemoryError: If this option is enabled, when an
out-of-memory error occurs, the JVM crashes and produces text and
binary
crash files.
The term "crash" is not very appropriate - crashes are bad things.
Abort
may have been a better choice.
For more details please refer http://ccc.us.oracle.com/8138745
This is not accessible outside of Oracle.
A few minor comments:
src/share/vm/runtime/globals.hpp
+ "JVM crashes and produces text and binary crash files")
Terminology should be consistent with other options that control core
dump. Should also say "on first occurrence of an out-of-memory error".
src/share/vm/utilities/debug.cpp:
308 if (CrashOnOutOfMemoryError) {
309 tty->print_cr("java.lang.OutOfMemoryError: %s", message);
310 fatal("OutOfMemory encountered: %s", message);
311 }
don't really need the tty->print when using the fatal. Though you may
want to use the j.l.OOME form of the message for consistency. Might
also
be useful for both Crash and Exit to include in the logged messages
the
fact that these flags were involved - something like:
"Terminating due to java.lang.OutOfMemoryError: %s"
"Aborting due to java.lang.OutOfMemoryError: %s"
Thanks,
David
-----
Regards,
Cheleswer
