jmx-dev RFR 8139870: sun.management.LazyCompositeData.isTypeMatched() fails for composite types with items of ArrayType

[Jaroslav Bachorik]

On 19.10.2015 16:28, Daniel Fuchs wrote:
> On 19/10/15 15:36, Jaroslav Bachorik wrote:
>> On 19.10.2015 14:40, Daniel Fuchs wrote:
>>> Hi Jaroslav,
>>>
>>> This - and the cleanup - look good to me, but it would
>>> be nicer if it was accompanied by a unit test :-)
>>
>> This time with test -
>> http://cr.openjdk.java.net/~jbachorik/8139870/webrev.01
>
> Thanks for the test Jaroslav!
>
>> Testing *only* the correctness of the newly introduced functionality
>> (eg. the test fails on older JDKs and passes on the build with the fix
>> in place). Providing full coverage for LazyCompositeData is probably out
>> of the scope of this change.
>
> Yes, I agree. This is fine.
> BTW - did you verify that the JCK still passes?
> Otherwise you might need to challenge it...

Not yet. I really hope it will pass - otherwise it would mean that the 
incorrect behaviour has been baked in :/

-JB-

>
> best regards,
>
> -- daniel
>
>>
>> -JB-
>>
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> On 19/10/15 13:37, Jaroslav Bachorik wrote:
>>>> Please, review the following change
>>>>
>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8139870
>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8139870/webrev.00
>>>>
>>>> sun.management.LazyCompositeData.isTypeMatched() method is used to
>>>> compare two composite types with the backward compatibility in mind.
>>>> The
>>>> idea is that when we have two types (type1, type2) type1 is matched by
>>>> type2 when and only when type2 contains all the items of type1 and
>>>> their
>>>> types are in turn matching the item types from type1, recursively.
>>>>
>>>> However, this method fails to account for ArrayType type and instead of
>>>> calling isTypeMatched() recursively on the array type it performs plain
>>>> Object.equals(). This will cause problems when one tries to safely
>>>> evolve (only adding items) composite types referred through ArrayType
>>>> items.
>>>>
>>>> The patch adds the missing functionality.
>>>>
>>>> In http://cr.openjdk.java.net/~jbachorik/8139870/webrev.00/cleanup
>>>> there
>>>> is a followup webrev of code warnings cleanup for
>>>> s.m.LazyComponentData.
>>>>
>>>> Thanks,
>>>>
>>>> -JB-
>>>
>>
>