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.par...@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.par...@oracle.com



--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com

Reply via email to