Request for Review (XXL): 7104647: Adding a diagnostic command framework

[Mandy Chung]

On 12/7/2011 1:30 PM, Frederic Parain wrote:
>
>> The execute method doesn't throw any exception other than
>> IAE. What happens if the specified command execution fails
>> in the target VM? If no exception is thrown, how does the
>> caller detect if the command succeeds or not and handle it
>> gracefully? It seems that this mechanism should provide
>> a way to detect if a command succeeds or fails programmatically
>> rather than burying the error message in the returned string.
>
> This changeset contains the framework, and the framework
> only throws NPE or IAE. However, a diagnostic command
> impementation is free to throw any other exception. If such
> an exception is thrown, it is simply printed on the output
> stream if the command has been invoked through the attachAPI,
> or it is propagated like any other exception through JMX
> if the command has been invoked from the HotSpotDiagnosticMXBean.
>

What happens when HotSpotDiagnosticMXBean is called locally within the 
same process but through JMX MBeanServer?

I still think you might need DiagnosticCommandException (something like 
that).

>
>> L133: do you need to check if command == null and throw NPE?
>> or NPE will be thrown somewhere?
>
> The native code is protected against null commands, it will
> throw the NPE.

Ok.  It's still good to add the null check to make this more explicit - 
you can use the java.util.Objects.requireNonNull method.

>
>> L158, 172: this should never happen unless the hotspot/jdk is
>> mismatched. But would it better to throw an exception with
>> an informative message to help diagnostic in case the mismatch
>> does happen?
>
> If a mismatch is detected, the implementation now throws an
> UnsupportedOperationException("Diagnostic commands are not supported 
> by this VM").
>

That's better.

Thanks
Mandy