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

[Aleksey Shipilev]

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