RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9

[mandy chung]

On 2/26/18 3:35 AM, Daniel Fuchs wrote:
> Hi Mandy,
>
> Nice to see this fixed.
>
> In ThreadInfoCompositeData::initV6CompositeType
>
>  425                     if (name.equals(STACK_TRACE)) {
>  426                         ot = new ArrayType<>(1, 
> StackTraceElementCompositeData.v5CompositeType());
>  427                     } else if (name.equals(LOCKED_MONITORS)) {
>  428                         ot = new ArrayType<>(1, 
> MonitorInfoCompositeData.v5CompositeType());
>
> This is probably OK for StackTraceElementCompositeData, but
> is confusing for MonitorInfoCompositeData. I thought
> MonitorInfo was added in 6? so why would it have a
> v5CompositeType?
>

Good catch!  I certainly don't like numbering them explicitly.  I will 
see if I can come up with some simple way to support future changes.

> I'd suggest renaming v5 to v6 in MonitorInfoCompositeData, and also
> adding a comment when calling 
> StackTraceElementCompositeData.v5CompositeType()
> to say that nothing changed in StackTraceElementCompositeData
> between 6 and 5.
>

Will fix this.   I will file a CSR for you to review.

thanks
Mandy
> best regards,
>
> -- daniel
>
>
>
> On 24/02/2018 00:50, mandy chung wrote:
>> Webrev at:
>> http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/webrev.01
>>
>> This patch updates the spec to clarify what attributes are required
>> since which release.  There is a spec bug that "classLoaderName"
>> added in JDK 9 is missing in the CompositeData for StackTraceElement
>> but the implementation is correct.  I will file a CSR for this update.
>>
>> This patch ensures that CompositeData for ThreadInfo of version N
>> must have the attributes defined since <= N.
>> ThreadInfo::from also makes sure 'stackTrace' and 'lockedMonitors'
>> attributes of version N while it can include additional attributes
>> which has been the current behavior.
>>
>> JDK-8139587 intended to support older versions of StackTraceElement
>> which does not seem a complete solution.  I reverted the fix for
>> JDK-8139587 (mostly) and removed TypeVersionMapper.  The fix constructs
>> the CompositeType for ThreadInfo and MonitorInfo of different
>> versions and used them for validation.  Minor cleanup: the static
>> final variables are renamed to all capitals. CompatibilityTest.java
>> test is missing the copyright header.
>>
>> thanks
>> Mandy
>> [1]http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/api/java/lang/management/ThreadInfo.html#from(javax.management.openmbean.CompositeData) 
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180226/ea202cd8/attachment.html>