RFR JDK-8028275: Metaspace ShrinkGrowTest causes fatal error if run with JFR

[David Holmes]

On 8/01/2014 2:59 AM, Coleen Phillimore wrote:
>
> I vote that we clean it up rather than adding a special exception mark.

Do you mean clean up the whole thing? That's a big(ger) job.

I'm not a fan of a special exception mark though as I think this would 
be the single usage for it - and unless we add a regular ExceptionMark 
to cover the code after is_init_completed() then we may as well just 
have moved the original scope as I suggested.

David

> Coleen
>
> On 01/07/2014 09:09 AM, Zhengyu Gu wrote:
>> I wonder a special INIT_EXCPTION_MARK, which always takes
>> is_init_completed() == false path, a better solution? We eventually
>> have to clean up the error handling code here if we want to recover
>> init failure from this routine.
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>
>> On Jan 6, 2014, at 9:49 PM, David Holmes wrote:
>>
>>> On 7/01/2014 6:48 AM, Zhengyu Gu wrote:
>>>> Hi Coleen,
>>>>
>>>> Thanks for the review.
>>>>
>>>> ExceptionMark's destructor only report pending exception as fatal
>>>> error after initialize is done (is_init_completed() == true),
>>>> otherwise, it will call vm_exit_during_initialization(), just as you
>>>> suggested.  Only after set_init_completed() is called (thread.cpp
>>>> #3568), pending exception has to be checked.
>>> Right - this subtlety is often overlooked. Though perhaps the simpler
>>> fix is to reduce the scope of the ExceptionMark so that it is
>>> "destructed" before is_init_completed() is called ? What you have is
>>> consistent with other error handling code so I'm okay with it as-is.
>>> Though a comment re CHECK and the ExceptionMark might be useful after
>>> set_init_completed().
>>>
>>> (All this initialization error handling is an awful mess regardless.
>>> It should be CHECK_JNIERR not CHECK_0.)
>>>
>>> David
>>>
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>
>>>>
>>>>
>>>> On Jan 6, 2014, at 3:37 PM, Coleen Phillimore wrote:
>>>>
>>>>> Zhengyu,
>>>>>
>>>>> There are other calls to initialize_class in Threads::create_vm()
>>>>> that can create a pending exception and return with CHECK_0 after
>>>>> the EXCEPTION MARK.
>>>>>
>>>>> It seems to me that the EXCEPTION_MARK should be removed and the
>>>>> caller of this function checks for the pending exception.   Better
>>>>> yet would be to encapsulate the calls to initialize_class into a
>>>>> function(s) and check the return for pending exception and exit the
>>>>> jvm with jvm_exit_during_initialization().
>>>>>
>>>>> Coleen
>>>>>
>>>>> On 01/06/2014 10:55 AM, Zhengyu Gu wrote:
>>>>>> This is another bug related on using EXCEPTION_MARK and CHECK
>>>>>> macros. JSR292 initialization is done after init_completed flag is
>>>>>> set to true, so any pending exceptions from initializing JSR292
>>>>>> classes can cause CHECK macro to return, and trigger the assertion
>>>>>> in ExceptionMark destructor.
>>>>>>
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8028275
>>>>>> Webrev: http://cr.openjdk.java.net/~zgu/8028275/webrev.00/
>>>>>>
>>>>>> Test:
>>>>>>    Tested the fix on reported platform.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Zhengyu
>