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

[David Holmes]

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