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

[harold seigel]

Hi Per,

Thank you for the comments.  I have incorporated suggested changes in a 
new webrev that is available for review at 
http://cr.openjdk.java.net/~hseigel/bug_8011773_2/ 
<http://cr.openjdk.java.net/%7Ehseigel/bug_8011773_2/>. Also, please see 
my comments inline.

Thanks, Harold

On 4/25/2013 9:57 AM, Per Lidén wrote:
> Hi,
>
> Some comments (not a reviewer):
>
> * lookup() is static, so StringTable::the_table()->lookup() should be 
> StringTable::lookup(). Not sure why the_table() is public, seems odd.
Done.
> * Wouldn't it be nicer if lookup(jchar* name, int len) was lookup(oop 
> string) and let the lookup function worry about the reset?
It may be nicer but that cleanup may not be worth the benefits.
>
> * Can't we just avoid allocating memory in as_unicode_string() and 
> instead just do something like:
>
> return length == 0 ? NULL : value->char_at_addr(offset);
>
> Of course, that would require some adjustments to the inner 
> String::intern(), but nothing major as far as I can tell.
That would simplify as_unicode_string(), but wouldn't that just move the 
memory allocation to the String::intern() method?
>
> cheers,
> /Per
>
> On 2013-04-24 17:26, 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
>