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

On 28/02/18 18:08, mandy chung wrote: Hi Daniel, The from method validates the CompositeType regardless of the attribute value is null or not. In addition, the CompositeData also  I updated the test to make sure that's the case. I tweak the wording s/composite data/composite type/ to avoid

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

Hi Daniel, The from method validates the CompositeType regardless of the attribute value is null or not. In addition, the CompositeData also  I updated the test to make sure that's the case. I tweak the wording s/composite data/composite type/ to avoid the ambiguity. * The {@code

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

Hi Mandy, This looks very good. In the API documentation of ThreadInfo::from, below the table that lists the attributes of StackTraceElement, I wonder if the following text should be added for completeness: ```A CompositeData representing a MonitorInfo of version N must contain a

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

On 2/27/18 10:05 PM, Jeremy Manson wrote: Yup, that's better.  I'd probably say "The same rule" instead of "Same rule". Thanks. Will fix it before I push. Mandy Jeremy On Tue, Feb 27, 2018 at 10:55 AM, mandy chung > wrote:

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

Yup, that's better. I'd probably say "The same rule" instead of "Same rule". Jeremy On Tue, Feb 27, 2018 at 10:55 AM, mandy chung wrote: > Good point, Jeremy. I notice some strange-ness when I wrote it but > wasn't able to pin point the error. Daniel also suggests to

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

On 2/27/18 9:44 AM, Jeremy Manson wrote: On Mon, Feb 26, 2018 at 4:53 PM, mandy chung > wrote: On 2/26/18 4:23 PM, Jeremy Manson wrote: Hi Mandy, Thanks for taking this on!  I'm happy to see that you are happy to

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

I made further edits to the javadoc and I am happy with this version (move out the attributes for StackTraceElement as a separate table). Specdiff: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/specdiff/overview-summary.html javadoc:

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

Good point, Jeremy. I notice some strange-ness when I wrote it but wasn't able to pin point the error. Daniel also suggests to clarify MonitorInfo as well. Does this version look better? * Returns a {@code ThreadInfo} object represented by the * given {@code CompositeData}.

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

CSR: https://bugs.openjdk.java.net/browse/JDK-8198732 Can you please review? Mandy On 2/23/18 4:50 PM, 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.

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

On 2/26/18 4:23 PM, Jeremy Manson wrote: Hi Mandy, Thanks for taking this on!  I'm happy to see that you are happy to do cleanups I was too timid to do (like adding the Factory in the tests). I note a few places in the test code where static initializers can throw RuntimeExceptions.  When

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

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   

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

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

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

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

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

Hi Jeremy, I have revised the spec of ThreadInfo::from to include "since" column: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/api/java/lang/management/ThreadInfo.html#from(javax.management.openmbean.CompositeData) I caught a few other issues like mix-n-match older version of

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

On 2/20/18 9:48 PM, Jeremy Manson wrote: I think that's a much better approach (I didn't notice the validate method :) ) I think you may want to grab my test changes, or make some similar ones.  Presumably, the tests do not pass with your change. I ran your test and it passed before

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

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:

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

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

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

Hi Jeremy, I'm not sure I understand exactly what is going on there. I wonder if this has anything to do with an issue I noticed some time back when the jvisualvm for JDK 8 and JDK 9 were still out of sync: https://bugs.openjdk.java.net/browse/JDK-8167099 I stumbled on that because at the time

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

On 16/02/2018 11:12 AM, Jeremy Manson wrote: Previous bug: https://bugs.openjdk.java.net/browse/JDK-6588467 And the review thread: http://mail.openjdk.java.net/pipermail/serviceability-dev/2015-January/016356.html I don't think the bug would have been obvious to a reviewer (or, indeed, the

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

Hi Jeremy, On 16/02/2018 10:46 AM, 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 Was there