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)
