Request for review: 8011773: Some tests on Interned String crashed JVM with OOM

[David Holmes]

On 25/04/2013 1:57 AM, Coleen Phillimore wrote:
>
> Hi, I like this change - it's moving in the direction of not aborting
> the VM for every C heap memory allocation failure we encounter if the
> Java application can do something about it (ie catch OutOfMemoryError).

I agree with not aborting the VM but not so sure the application can 
really do something about it :)

> A couple of problems with the code:
>
> http://cr.openjdk.java.net/~hseigel/bug_8011773/src/share/vm/classfile/symbolTable.cpp.udiff.html
> <http://cr.openjdk.java.net/%7Ehseigel/bug_8011773/src/share/vm/classfile/symbolTable.cpp.udiff.html>
>
>
> The CHECK_NULL will return NULL with the exception in the Thread object
> so, you don't have to check for chars != NULL before calling intern. See
> utilities/exceptions.hpp for what these macros do.  They are not nice.

Similarly for the code in whitebox.cpp.

That said don't be surprised if parfait flags these if there is no 
explicit NULL check. :(

Cheers,
David

> http://cr.openjdk.java.net/~hseigel/bug_8011773/src/share/vm/oops/oop.cpp.udiff.html
> <http://cr.openjdk.java.net/%7Ehseigel/bug_8011773/src/share/vm/oops/oop.cpp.udiff.html>
>
>
> The CATCH at the end will assert that an exception wasn't thrown, which
> is not what you want.  I think you want to pass THREAD, and then keep
> the null check on the next line.   For this situation, I think it's
> reasonable to call vm_exit_out_of_memory() since that is existing
> behavior and this is for rehashing the string table in a loop and
> there's a resource mark so it's difficult to run out of C heap memory
> while doing this.
>
> Thanks,
> Coleen
>
> On 4/24/2013 11:26 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this fix for bug 8011773.
>>
>> Summary: Method as_unicode_string() now detects the memory allocation
>> failure and throws an OutOfMemoryError exception.  This lets its
>> callers decide whether to abort the VM or allow the exception to be
>> caught and execution continue.
>>
>> Open webrev at: http://cr.openjdk.java.net/~hseigel/bug_8011773/
>> <http://cr.openjdk.java.net/%7Ehseigel/bug_8011773/>
>> Bug Link at: http://bugs.sun.com/view_bug.do?bug_id=8011773
>>
>> Regression testing was done using JCK lang and vm tests, jtReg tests,
>> JPRT, and ute vm.quick.testlist tests.  Also, as_unicode_string() was
>> temporarily changed to throw OutOfMemoryError exceptions in order to
>> verify that the expected behavior occurred.
>>
>> Thanks, Harold
>