RFR (XS) 8059924: com/sun/management/DiagnosticCommandMBean/DcmdMBeanPermissionsTest.java: assert(Universe::verify_in_progress() || !SafepointSynchronize::is_at_safepoint()) failed: invariant

[Aleksey Shipilev]

Thanks, Harold!

-Aleksey.

On 10.10.2014 16:14, harold seigel wrote:
> Hi Aleksey,
> 
> You have enough reviewers.
> 
> I'll sponsor the changeset.
> 
> Harold
> 
> On 10/10/2014 4:22 AM, Aleksey Shipilev wrote:
>> Anyone?
>>
>> Thanks,
>> -Aleksey.
>>
>> On 10/09/2014 04:43 PM, Aleksey Shipilev wrote:
>>> Keith, Lois, thanks for review!
>>>
>>> I think I need another Runtime Reviewer?
>>>
>>> Definitely need a sponsor. Here is the changeset:
>>>   http://cr.openjdk.java.net/~shade/8059924/8059924.changeset
>>>
>>> Thanks,
>>> -Aleksey.
>>>
>>> On 10/09/2014 03:39 PM, Lois Foltan wrote:
>>>> Aleksey, this looks good.
>>>> Lois
>>>>
>>>> On 10/9/2014 3:21 AM, Aleksey Shipilev wrote:
>>>>> Hi,
>>>>>
>>>>> There is a regression introduced by:
>>>>>    https://bugs.openjdk.java.net/browse/JDK-8059595
>>>>>
>>>>> The regression is tracked here:
>>>>>    https://bugs.openjdk.java.net/browse/JDK-8059924
>>>>>
>>>>> ...and here is a fix:
>>>>>    http://cr.openjdk.java.net/~shade/8059924/webrev.00/
>>>>>
>>>>> Testing: JPRT, jdk/test/com/sun/management
>>>>>
>>>>> The comment there should be self-explanatory. The failure requires a
>>>>> precise sequence of events to happen: we should be at safepoint, and
>>>>> call klass->external_name() on a previously untouched class. Before
>>>>> JDK-8059595, Verifier always did klass->external_name() for all
>>>>> verified
>>>>> classes, causing a performance trouble.
>>>>>
>>>>> This might explain why the failure happens on com/sun/management tests
>>>>> that do VM operations (GC_HeapInspection in reported failure case).
>>>>> This
>>>>> reproduces perfectly with DcmdMBeanPermissionsTest, and the test is
>>>>> fixed with the VM change above.
>>>>>
>>>>> The alternative would be to backout the JDK-8059595 change completely,
>>>>> but this gets us back to weird piggybacking on klass->external_name()
>>>>> during the verification. Other alternatives: returning 0 from hashcode
>>>>> during safepoint is obviously unsafe; returning zero-hashcode string
>>>>> from klass->external_name() is potentially unsafe if someone is using
>>>>> the name as the map key downstream.
>>>>>
>>>>> -Aleksey.
>>
>