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

[Daniel Fuchs]

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

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