RFR: 8004095: Add support for JMX interface to Diagnostic Framework and Commands,
Staffan Larsen
staffan.larsen at oracle.com
Mon Dec 17 07:47:34 PST 2012
diagnosticCommand.cpp:259: nit: wrong indentation
Otherwise: Looks good to me!
Thanks,
/Staffan
On 17 dec 2012, at 16:01, Frederic Parain <frederic.parain at oracle.com> wrote:
> Staffan,
>
> Thank you for the review, I've fixed the code to address all
> the issue you reported. The new webrev is here:
>
> http://cr.openjdk.java.net/~fparain/8004095/webrev.01/
>
> Regards,
>
> Fred
>
> On 12/17/12 01:27 PM, Staffan Larsen wrote:
>> Frederic,
>>
>> Looks good! Just a few small comments below.
>>
>> diagnosticCommand.cpp:255: When DisableExplicitGC is set, it would be great if the SystemGC command printed on error message so that the caller knows that the GC didn't happen as intended. I also think we should add an entry to GCCause for GCs caused by the DCmd (but that isn't new to this change).
>>
>> diagnosticFramework.hpp:423: nit: misspelled "enabled" in the method name
>>
>> diagnosticFramework.cpp:435: seems some testing code has slipped in
>>
>> diagnosticFramework.cpp:509: nit: missing spaces around '&'. (Consider adding () to clarify the precedence.)
>>
>> Thanks,
>> /Staffan
>>
>> On 17 dec 2012, at 12:26, Frederic Parain <frederic.parain at oracle.com> wrote:
>>
>>> Hi,
>>>
>>> Please review the following change for CR 8004095.
>>>
>>> This changeset is the JDK side of two parts project.
>>>
>>> The goal of this feature is to allow remote invocations of
>>> Diagnostic Commands via JMX.
>>>
>>>
>>> Diagnostic Commands are actually invoked from the jcmd
>>> tool, which is a local tool using the attachAPI. It was
>>> not possible to remotely invoke these commands.
>>> This project creates a new PlatformMBean, remotely
>>> accessible via the Platform MBean Server, providing
>>> access to Diagnostic Commands too.
>>>
>>> The JDK side of this project, including the new
>>> Platform MBean, is covered in CR 7150256.
>>>
>>> This changeset contains the modifications in the
>>> JVM to support the new API.
>>>
>>> There are two types of modifications:
>>> 1 - extension of the diagnostic command framework
>>> 2 - extension of existing diagnostic commands
>>>
>>> Modifications of the framework:
>>>
>>> The first modification of the framework is the mechanism
>>> to communicate with the DiagnosticCommandsMBean defined
>>> in the JDK. Communications are performed via the jmm.h
>>> interface. In addition to a few new entries in the
>>> jmm structure, several data structures have been declared
>>> to export diagnostic command meta-data to the JDK.
>>>
>>> The second modification of the framework consists in an
>>> export control mechanism. Diagnostic Commands are executed
>>> inside the JVM, they have access to critical information
>>> and can perform very disruptive operations. Even if the
>>> DiagnosticCommandsMBean includes some security checks
>>> using Java Permissions, it sometime better to simply
>>> make a diagnostic command not available from the JMX
>>> API. The export control mechanism gives to diagnostic
>>> command developers full control on how the command will
>>> be accessible. When a diagnostic command is registered
>>> to the framework, a list of interfaces allowed to
>>> export this command must be provided. The current
>>> implementation defines three interfaces: JVM internal,
>>> attachAPI and JMX. A diagnostic command can be
>>> exported to any combination of these interfaces.
>>>
>>> Modifications of existing diagnostic commands:
>>>
>>> Because this feature allows remote invocations, it
>>> is important to be able to control who is invoking
>>> and what is invoked. Java Permission checks have
>>> been introduced in the DiagnosticCommandsMBean and
>>> are performed before a remote invocation request
>>> is forwarded to the JVM. So, each Diagnostic Command
>>> can require a Java Permission to be invoked remotely.
>>> Note that invocations from inside the JVM or via the
>>> attachAPI do not check these permissions. Invocations
>>> from inside the JVM are considered trusted, and the
>>> attachAPI has its own security model based on EUID/
>>> EGID.
>>> Some existing diagnostic commands that have been
>>> identified as begin potentially harmful have been
>>> extended to specify the Java Permission required
>>> for their execution.
>>>
>>> The webrev is here:
>>>
>>> http://cr.openjdk.java.net/~fparain/8004095/webrev.00/
>>>
>>> Thanks,
>>>
>>> Fred
>>>
>>>
>>>
>>>
>>>
>>> --
>>> Frederic Parain - Oracle
>>> Grenoble Engineering Center - France
>>> Phone: +33 4 76 18 81 17
>>> Email: Frederic.Parain at Oracle.com
>>>
>>
>
> --
> Frederic Parain - Oracle
> Grenoble Engineering Center - France
> Phone: +33 4 76 18 81 17
> Email: Frederic.Parain at Oracle.com
>
More information about the serviceability-dev
mailing list