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

[harold seigel]

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