[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9
From:       Daniel Fuchs <daniel.fuchs () oracle ! com>
Date:       2018-02-28 18:46:34
Message-ID: e144bf5c-e15f-e107-e2d0-9d998fc7119e () oracle ! com
[Download RAW message or body]

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 the ambiguity.
>   
>       * The {@code "lockedStackFrame"} attribute in
>       * {@link MonitorInfo#from(CompositeData) MonitorInfo}'s composite type
>       * must represent {@code StackTraceElement} of the same version <em>N</em>.

Looks fine!

-- daniel

> 
> Mandy
> 
> On 2/28/18 3:07 AM, Daniel Fuchs wrote:
>> 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 lockedStackTrace attribute that is either null if 
>> stackDepth   < 0, or is a CompositeData representing a 
>> StackTraceElement of version N.```
>>
>> What do you think?
>>
>> best regards,
>>
>> -- danie
>>
>> On 27/02/2018 20:31, mandy chung wrote:
>>> 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:
>>> http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/api/java/lang/management/ 
>>>
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/webrev.02/
>>>
>>> Mandy
>>>
>>> On 2/27/18 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 clarify
>>>> MonitorInfo as well.
>>>>
>>>> Does this version look better?
>>>>
>>>>            * Returns a {@code ThreadInfo} object represented by the
>>>>            * given {@code CompositeData}.
>>>>            * <a id="attributes"></a>
>>>>            * A {@code CompositeData} representing a {@code ThreadInfo} of
>>>>            * version <em>N</em> must contain all of the attributes defined
>>>>            * in version &le; <em>N</em> unless specified otherwise.
>>>>            * Same rule applies transitively to attributes whose type or
>>>>            * component type is {@code CompositeType}.
>>>>            * <p>
>>>>            * A {@code CompositeData} representing {@code ThreadInfo} of 
>>>> version
>>>>            * <em>N</em> contains {@code "stackTrace"} attribute representing
>>>>            * an array of {@code StackTraceElement} of version <em>N</em>.
>>>>            * The {@code "lockedMonitors"} attribute represents
>>>>            * an array of {@link MonitorInfo} of version <em>N</em>
>>>>            * which implies that its {@code "lockedStackFrame"} attribute 
>>>> also
>>>>            * represents {@code StackTraceElement} of the same version, 
>>>> <em>N</em>.
>>>>            * Otherwise, this method will throw {@code 
>>>> IllegalArgumentException}.
>>>>
>>>> Mandy
>>>
>>
> 

[prev in list] [next in list] [prev in thread] [next in thread]