review (XS) for 7018506: CTW swallowing OOMs after symbol changes

[Tom Rodriguez]

On Feb 10, 2011, at 11:57 AM, Vladimir Kozlov wrote:

> Tom,
> 
> I think new method should return boolean if exception occurred.
> There is checked variable 'exception_occurred' to which it
> should be assigned to:
> 
> 1324       bool exception_occurred = clear_pending_exception_if_not_oom(CHECK);

I considered that but the natural way to use that is:

                if (clear_pending_exception_if_not_oom(CHECK)) {

but having CHECK macros in conditional doesn't work right.  Also I think the logic only wants to skip if something went wrong with the initial class load and initialize, not with the constant pool preload_and_init part.

tom

> 
> 1325       if (CompileTheWorldPreloadClasses && k.not_null()) {
> 1326         constantPoolKlass::preload_and_initialize_all_classes(k->constants(), THREAD);
> 1327         if (HAS_PENDING_EXCEPTION) {
> 1328           // If something went wrong in preloading we just ignore it
> 1329           exception_occurred = clear_pending_exception_if_not_oom(CHECK);
> 1330           tty->print_cr("Preloading failed for (%d) %s", _compile_the_world_counter, buffer);
> 1331         }
> 1332       }
> 
> Thanks,
> Vladimir
> 
> Tom Rodriguez wrote:
>> Coleen pointed out there are other unchecked CLEAR_PENDING_EXCEPTION calls so I've modified it to check at all of them.  Please re-review.
>> tom
>> On Feb 10, 2011, at 11:36 AM, Igor Veresov wrote:
>>> Looks good.
>>> 
>>> igor
>>> 
>>> On 2/10/11 11:13 AM, Tom Rodriguez wrote:
>>>> http://cr.openjdk.java.net/~never/7018506
>>>> 
>>>> 7018506: CTW swallowing OOMs after symbol changes
>>>> Reviewed-by:
>>>> 
>>>> Previously the symbol creation could throw an OOM for perm but now it
>>>> can't and the next two exception points just swallowed exception so it
>>>> keeps on going.  The fix is to always allow OOMs to be propagated.
>>>> Tested with CTW.