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

[Per Lidén]

Hi Harold,

(comment inline)

On 2013-04-26 15:34, harold seigel wrote:
> 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?

I guess there would be two alternatives:

1) Move the allocation into String::intern(), which would mean that 
other users of as_unicode_string() don't need to worry about allocations 
and exceptions.
2) Don't allocate anything. In String::intern() just make sure the array 
oop kept in a Handle across possible safepoints. The array from 
as_unicode_string() is only used for reading anyway (hash calculation, etc).

Feel free to keep your patch as is. It just seemed like a nice idea to 
remove the allocation that was causing problems, since as far as I can 
tell it's not strictly unnecessary.

cheers,
/Per

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