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

[mandy chung]

Hi Jeremy,

Another approach is to change ThreadInfoCompositeData::validateCompositeData
to validate the given CompositeData.   I also revised 
ThreadInfoCompositeData to
return the default value of the attributes, if not present.

This is the patch:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/webrev.00/

What do you think?
Mandy

On 2/20/18 4:46 PM, Jeremy Manson wrote:
> (I dropped serviceability-dev from this thread by mistake! Oops!)
>
> Okay. Here's the revised patch.  LMK if that's what you had in mind.
>
> http://cr.openjdk.java.net/~jmanson/8198253/webrev.01/ 
> <http://cr.openjdk.java.net/%7Ejmanson/8198253/webrev.01/>
>
> On Fri, Feb 16, 2018 at 6:20 PM, mandy chung <mandy.chung at oracle.com 
> <mailto:mandy.chung at oracle.com>> wrote:
>
>     Hi Jeremy,
>
>     lockedMonitors and lockedSynchronizers attribute are not optional if that's
>     the issue you try to resolve. I think the specification should be clarified.
>
>     ThreadInfo::from supports the three different versions for interoperability:
>     1. CompositeData for JDK 1.5 ThreadInfo with no lockedMonitors and
>         lockedSynchronizers attribute
>     2. CompositeData for JDK 6 ThreadInfo with lockedMonitors and lockedSynchronizers
>         attributes
>     3. CompositeData for JDK 9 ThreadInfo with lockedMonitors and lockedSynchronizers
>         attributes and with daemon and priority attribute.
>
>     JMX client can connect to a running VM in any version and get back a
>     proper ThreadInfo.
>
>     If ThreadInfo::from is called with a CompositeData containing daemon and
>     priority attribute but lockedMonitors and lockedSynchronizers attributes are
>     absent then the given CompositeData is invalid and IllegalArgumentException
>     should be thrown.
>
>     Does this help?
>
>     Mandy
>
>
>     On 2/15/18 4:46 PM, Jeremy Manson wrote:
>>     Hi folks,
>>
>>     Been a long time!  I'd like someone to do a code review.  This is
>>     in code I wrote a few years ago, and got wrong.  At the time,
>>     David Holmes, Staffan Larsen, and Mandy Chung reviewed it.  It
>>     does mean that people using ThreadInfo.from(CompositeData) may be
>>     getting the wrong values out for ThreadInfo, so it is definitely
>>     worth fixing.
>>
>>     The patch below fixes the bug, but is a pretty questionable
>>     approach.  Better approaches happily considered.
>>
>>     Patch:
>>     http://cr.openjdk.java.net/~jmanson/8198253/webrev.00/
>>     <http://cr.openjdk.java.net/%7Ejmanson/8198253/webrev.00/>
>>
>>     Bug:
>>     https://bugs.openjdk.java.net/browse/JDK-8198253
>>     <https://bugs.openjdk.java.net/browse/JDK-8198253>
>>
>>     Thanks!
>>
>>     Jeremy
>
>

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