RFR[ 9u-dev] JDK-8138745: Implement ExitOnOutOfMemory and CrashOnOutOfMemory in HotSpot

[David Holmes]

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
>>>>>>
>>>
>