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

[Jeremy Manson]

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 I ran the tests, and a static initializer threw a
RuntimeException, I didn't see it reflected in the output, so I had to add
print statements.  Was I just doing it wrong, or is that a feature of jtreg?

Just a couple of random nits:

>  267      * @throws IllegalArgumentException if the given CompositeData
does not
>  268      * contain all attributes representing ThreadInfo of any release.

Could this be misinterpreted?  If you have everything in V5, and only one
of the V6 attributes, it contains all of the attributes representing the
ThreadInfo of V5.  Maybe you want to say "does not contain exactly the
attributes present in ThreadInfo for a particular release"?

For signatures like:

> static CompositeType initV5CompositeType(CompositeType ctype) {

I probably would change the name of ctype to indicate when you are reading
the method that the parameter is always supposed to be the the
CompositeType for ThreadInfo.




On Fri, Feb 23, 2018 at 4:50 PM, mandy chung <mandy.chung at oracle.com> 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/c1ecf974/attachment-0001.html>