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

[Coleen Phillimore]

Ok, I added the comment.  Thank you for the code review!!
Coleen

On 01/19/2014 09:51 PM, David Holmes wrote:
> On 17/01/2014 11:07 PM, Coleen Phillimore wrote:
>> On 1/16/2014 11:52 PM, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> On 17/01/2014 8:46 AM, Coleen Phillimore wrote:
>>>> Summary: Clean up initialization from Threads::create_vm() so that
>>>> exceptions cause vm_exit_during_initialization without an exception
>>>> mark.
>>>>
>>>> My attempt at cleaning up initialization code paths that can throw an
>>>> exception.  I only cleaned up the code in Threads::create_vm(). There
>>>> are other places where EXCEPTION_MARK can occur after
>>>> Universe::is_fully_initialized() and before 
>>>> is_initialization_complete()
>>>> deeply embedded in the code.   But I did some refactoring.
>>>
>>> Why do you check
>>>
>>> 5210     if (Universe::is_fully_initialized()) {
>>
>> I can't get the Thread::current() unless the universe is initialized (or
>> rather that's the check to prevent me from getting it too soon).
>
> Ah and I was overlooking what HAS_PENDING_EXCEPTION expands into.
>
>>>
>>> before calling vm_exit_during_initialization? If we have a pending
>>> exception shouldn't we be exiting that way regardless. Otherwise what
>>> do we do if we have a pending exception but the universe is not
>>> initialized?
>>
>> There won't be a pending exception without the universe initialized
>> because (more secret code) Exceptions::_throw() will do a
>> vm_exit_during_initialization() if that's the case - can't create an
>> exception object yet.
>
> In that case can you add a comment like:
>
> // else: no pending exception possible - VM will already have aborted
>
> Thanks,
> David
>
>>>
>>> In threads.cpp you dropped the "if (EnableInvokeDynamic) " guard on
>>> the jsr292 class initialization.
>>>
>> Oops, will add that back.
>>> Otherwise I like the way you now use CHECK_JNIERR everywhere, even
>>> though we still abort in CreateJavaVM :) One step at a time.
>>>
>>
>> Yes, this has to be incremental.   thanks!
>>
>> Coleen
>>> Thanks,
>>> David
>>> -----
>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8028275/
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8028275
>>>>
>>>> Tested with nsk.quick.testlist and with the hotspot jtreg tests, and
>>>> some custom code to test exception paths.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>