Hi Daniel,
> On Apr 29, 2016, at 8:08 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>
> Hi,
>
> Please find below a patch [2] that eliminates a static
> dependency of java.lang.management on java.util.logging.LoggingMXBean.
>
> When JDK-6876135 was fixed, it introduced the PlatformLoggingMXBean
> interface, and recommended using PlatformLoggingMXBean over
> LoggingMXBean.
Adding to this, JDK-6876135 prepared the JDK for modularization and
PlatformLoggingMXBean was introduced that can be replaced with existing usage
of LoggingMXBean.
With this change, java.management dependency on java.logging will become
implementation details for logging purpose and can be eliminated completely in
the future.
About the deprecation, to be specific, LoggingMXBean will no longer be a
platform MXBean and an instance of LoggingMXBean will not register in the
platform MBeanServer.
This is a revised wording for the deprecation note for LoggingMXBean:
@deprecated {@code LoggingMXBean} is no longer a {@link
java.lang.management.PlatformManagedObject platform MXBean} and replaced with
{@link java.lang.management.PlatformLoggingMXBean}.
It will not register in the platform MBeanServer. Use
{@code ManagementFactory.getPlatformMXBean(PlatformLoggingMXBean.class)}
instead.
One question about: ManagementFactory::getPlatformMXBean(MBeanServerConnection,
PlatformLoggingMXBean.class)
- what would happen if this method is called from an image with java.logging
module present and connect to a VM with no java.logging module? Should the
ManagementFactory::getPlatformMXBean spec or PlatformLoggingMXBean spec be
clarified?
ManagementFactoryHelper.java
191 if (result != null) {
192 LoggingMXBeanSupport.class.getModule().addReads(m);
193 }
Reflection access assumes readability now:
http://openjdk.java.net/projects/jigsaw/spec/issues/#ReflectionWithoutReadability
So this addReads is not needed.
252 final Map<String, Method> methods = AccessController.doPrivileged(
253 new PrivilegedAction<>() {
254 @Override
255 public Map<String, Method> run() {
256 return initMethodMap(impl);
257 }
258 });
I believe this doPrivileged is not necessary and so do the other getMethod
calls. Probably you are thinking ahead what if java.management may one day be
defined by a child loader of the defining loader of java.logging. Then you can
move doPrivileged to initMethodMap.
217 // skip
- better to throw InternalError since it expects all methods are used?
273 throw new SecurityException(ex);
- should not reach here. SecurityException may cause confusion since this will
be thrown even without security manager. could simply throw InternalException
296 private PlatformLoggingImpl(LoggingMXBeanSupport support) {
- perhaps renaming LoggingMXBeanSupport to LoggingProxy to better represent it.
>
> Backward Compatibility considerations:
> --------------------------------------
>
> 1. Local clients which obtain an instance of the logging
> MXBean by calling ManagementFactory.getPlatformMXBean(
> "java.util.logging:type=Logging",
> PlatformLoggingMXBean.class)
> will no longer be able to cast the result on
> java.util.logging.LoggingMXBean.
> [There should be few, given that PlatformLoggingMXBean
> already has all the methods defined in LoggingMXBean]
>
I expect this would be really rare too.
> 2. ManagementFactory.getPlatformMBeanServer().isInstanceOf(
> ObjectName, "java.util.logging.LoggingMXBean")
> will now return 'false' instead of 'true'.
>
> 3. The Logging MXBean MBeanInfo will now report that its
> management interface is "java.lang.management.PlatformLoggingMXBean"
> instead of "sun.management.ManagementFactoryHelper$LoggingMXBean”.
Any impact to permission confiugred in security policy? Should also document
that.
> 4. Calls to ManagementFactory.newPlatformMXBeanProxy(
> MBeanServerConnection, ObjectName,
> java.util.logging.LoggingMXBean.class); and
> JMX.newMXBeanProxy(MBeanServerConnection, ObjectName,
> java.util.logging.LoggingMXBean.class)
> will continue to work as before.
>
> 5. Remote clients running previous version of the JDK
> should see no changes, except for the interface
> name in MBeanInfo, and the change in isInstanceOf
> reported in 2.
This is good.
The incompatibility risk for this change is rather low.
Mandy
