On 8/8/17 1:27 AM, Ujwal Vangapally wrote:

below is the link to new webrev incorporating review comments.

webrev: http://cr.openjdk.java.net/~uvangapally/webrev/2017/8185003/webrev.02/

769 * The Behaviour is same as {@link #getThreadInfo(long[], boolean, boolean)}
770 * except that the stack trace depth is limited.
771 *

Alternatively you can copy the javadoc from getThreadInfo(long[], boolean, boolean) to the new method, saying:

Returns the thread info for each thread whose ID is in the input array {@code ids}, with stack trace of the specified maxinum number of elements and synchronization information.

getThreadInfo(long[], boolean, boolean) can replace most of the javadoc by saying: This is equivalent to calling:
getThreadInfo(ids, lockedMonitors, lockedSynchronizers, Integer.MAX_VALUE)

Same applies to dumpAllThreads.

I did look at grepcode and see any custom implementation of ThreadMXBean. There are cases that extends ThreadMXBean which mostly attempts to call com.sun.management.ThreadMXBean without referencing the type. They are not custom implementation of ThreadMXBean.

I agree with Daniel's suggestion to make the new methods with a default implementation throwing UOE. This might make existing implementation easier to migrate.


495 if(maxDepth < 0) { 500 return dumpThreads0(null, lockedMonitors, lockedSynchronizers,maxDepth);

missing space between if and ( and ,

   long line 138.  You can wrap it.

This changes the interface and so you should add JMM_VERSION_10 even we don't support mixing hotspot with of different version of JDK libraries.

   line 1163 is quite long line.  Can you wrap it?

jmm_DumpThreads - it should probably validate maxDepth as jmm_GetThreadInfo does.
verified that

MBeanServerConnection.invoke throws ReflectionException when invoked with a method that doesn't exist in remote MBean server.

UndeclaredThrowableException will be throwed when a client gets proxy of remote MBean server and calls a method that doesn't exist on remote Mbean server.

do we need to develop Automated tests for verifying above cases ?

It'd be useful unless there is existing JMX tests covering this scenario. This is more on JMX framework and it's okay to separate a new issue for adding these new tests.

We should re-read the specification and see if spec clarification is needed.


Reply via email to