From 9d7df9ae2cbe9962842240da4dd2919a438a24a4 Mon Sep 17 00:00:00 2001 From: dfuchs Date: Tue, 9 Sep 2008 17:01:45 +0200 Subject: [PATCH] 6745832: jmx namespaces: Some refactoring/commenting would improve code readability. Reviewed-by: emcmanus --- .../DefaultMBeanServerInterceptor.java | 4 +- .../jmx/interceptor/DispatchInterceptor.java | 94 ++++---- .../DomainDispatchInterceptor.java | 82 +++++-- .../NamespaceDispatchInterceptor.java | 40 ++-- .../classes/com/sun/jmx/mbeanserver/Util.java | 15 +- .../sun/jmx/namespace/DomainInterceptor.java | 83 +++---- .../sun/jmx/namespace/HandlerInterceptor.java | 64 ++--- .../sun/jmx/namespace/JMXNamespaceUtils.java | 21 +- .../jmx/namespace/NamespaceInterceptor.java | 4 - .../jmx/namespace/RoutingConnectionProxy.java | 55 ++--- .../RoutingMBeanServerConnection.java | 218 ++++++++++-------- .../com/sun/jmx/namespace/RoutingProxy.java | 148 ++++++++++-- .../sun/jmx/namespace/RoutingServerProxy.java | 52 ++--- .../javax/management/namespace/JMXDomain.java | 12 +- .../management/namespace/JMXNamespace.java | 38 +-- .../management/namespace/JMXNamespaces.java | 6 +- .../namespace/JMXRemoteNamespace.java | 49 ++-- .../MBeanServerConnectionWrapper.java | 1 - test/javax/management/namespace/Wombat.java | 7 +- 19 files changed, 579 insertions(+), 414 deletions(-) diff --git a/src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java b/src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java index 7d95a77fd..7faeb8727 100644 --- a/src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java +++ b/src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java @@ -2021,7 +2021,7 @@ public class DefaultMBeanServerInterceptor private void addJMXNamespace(JMXNamespace namespace, final ObjectName logicalName, final Queue postQueue) { - dispatcher.addNamespace(logicalName, namespace, postQueue); + dispatcher.addInterceptorFor(logicalName, namespace, postQueue); } /** @@ -2035,7 +2035,7 @@ public class DefaultMBeanServerInterceptor private void removeJMXNamespace(JMXNamespace namespace, final ObjectName logicalName, final Queue postQueue) { - dispatcher.removeNamespace(logicalName, namespace, postQueue); + dispatcher.removeInterceptorFor(logicalName, namespace, postQueue); } /** diff --git a/src/share/classes/com/sun/jmx/interceptor/DispatchInterceptor.java b/src/share/classes/com/sun/jmx/interceptor/DispatchInterceptor.java index 9e8625d16..4a79567fe 100644 --- a/src/share/classes/com/sun/jmx/interceptor/DispatchInterceptor.java +++ b/src/share/classes/com/sun/jmx/interceptor/DispatchInterceptor.java @@ -194,7 +194,7 @@ public abstract class DispatchInterceptor // found in the handlerMap. Note: there doesn't need to be an interceptor // for that key in the Map. // - public abstract String getHandlerKey(ObjectName name); + abstract String getHandlerKey(ObjectName name); // Returns an interceptor for that name, or null if there's no interceptor // for that name. @@ -277,7 +277,7 @@ public abstract class DispatchInterceptor // of JMXNamespace (or a subclass of it) is registered as an MBean. // This method is usually invoked from within the repository lock, // hence the necessity of the postRegisterQueue. - public void addNamespace(ObjectName name, N jmxNamespace, + public void addInterceptorFor(ObjectName name, N jmxNamespace, Queue postRegisterQueue) { final String key = getHandlerKey(name); validateHandlerNameFor(key,name); @@ -298,7 +298,7 @@ public abstract class DispatchInterceptor // of JMXNamespace (or a subclass of it) is deregistered. // This method is usually invoked from within the repository lock, // hence the necessity of the postDeregisterQueue. - public void removeNamespace(ObjectName name, N jmxNamespace, + public void removeInterceptorFor(ObjectName name, N jmxNamespace, Queue postDeregisterQueue) { final String key = getHandlerKey(name); final T ns; @@ -330,7 +330,7 @@ public abstract class DispatchInterceptor } // From MBeanServer - public ObjectInstance createMBean(String className, ObjectName name) + public final ObjectInstance createMBean(String className, ObjectName name) throws ReflectionException, InstanceAlreadyExistsException, MBeanRegistrationException, MBeanException, NotCompliantMBeanException { @@ -338,7 +338,7 @@ public abstract class DispatchInterceptor } // From MBeanServer - public ObjectInstance createMBean(String className, ObjectName name, + public final ObjectInstance createMBean(String className, ObjectName name, ObjectName loaderName) throws ReflectionException, InstanceAlreadyExistsException, MBeanRegistrationException, MBeanException, @@ -347,7 +347,7 @@ public abstract class DispatchInterceptor } // From MBeanServer - public ObjectInstance createMBean(String className, ObjectName name, + public final ObjectInstance createMBean(String className, ObjectName name, Object params[], String signature[]) throws ReflectionException, InstanceAlreadyExistsException, MBeanRegistrationException, MBeanException, @@ -357,7 +357,7 @@ public abstract class DispatchInterceptor } // From MBeanServer - public ObjectInstance createMBean(String className, ObjectName name, + public final ObjectInstance createMBean(String className, ObjectName name, ObjectName loaderName, Object params[], String signature[]) throws ReflectionException, InstanceAlreadyExistsException, @@ -368,42 +368,43 @@ public abstract class DispatchInterceptor } // From MBeanServer - public ObjectInstance registerMBean(Object object, ObjectName name) + public final ObjectInstance registerMBean(Object object, ObjectName name) throws InstanceAlreadyExistsException, MBeanRegistrationException, NotCompliantMBeanException { return getInterceptorForCreate(name).registerMBean(object,name); } // From MBeanServer - public void unregisterMBean(ObjectName name) + public final void unregisterMBean(ObjectName name) throws InstanceNotFoundException, MBeanRegistrationException { getInterceptorForInstance(name).unregisterMBean(name); } // From MBeanServer - public ObjectInstance getObjectInstance(ObjectName name) + public final ObjectInstance getObjectInstance(ObjectName name) throws InstanceNotFoundException { return getInterceptorForInstance(name).getObjectInstance(name); } // From MBeanServer - public Set queryMBeans(ObjectName name, QueryExp query) { - final QueryInterceptor mbs = + public final Set queryMBeans(ObjectName name, + QueryExp query) { + final QueryInterceptor queryInvoker = getInterceptorForQuery(name); - if (mbs == null) return Collections.emptySet(); - else return mbs.queryMBeans(name,query); + if (queryInvoker == null) return Collections.emptySet(); + else return queryInvoker.queryMBeans(name,query); } // From MBeanServer - public Set queryNames(ObjectName name, QueryExp query) { - final QueryInterceptor mbs = + public final Set queryNames(ObjectName name, QueryExp query) { + final QueryInterceptor queryInvoker = getInterceptorForQuery(name); - if (mbs == null) return Collections.emptySet(); - else return mbs.queryNames(name,query); + if (queryInvoker == null) return Collections.emptySet(); + else return queryInvoker.queryNames(name,query); } // From MBeanServer - public boolean isRegistered(ObjectName name) { + public final boolean isRegistered(ObjectName name) { final MBeanServer mbs = getInterceptorOrNullFor(name); if (mbs == null) return false; else return mbs.isRegistered(name); @@ -415,20 +416,21 @@ public abstract class DispatchInterceptor } // From MBeanServer - public Object getAttribute(ObjectName name, String attribute) + public final Object getAttribute(ObjectName name, String attribute) throws MBeanException, AttributeNotFoundException, InstanceNotFoundException, ReflectionException { return getInterceptorForInstance(name).getAttribute(name,attribute); } // From MBeanServer - public AttributeList getAttributes(ObjectName name, String[] attributes) + public final AttributeList getAttributes(ObjectName name, + String[] attributes) throws InstanceNotFoundException, ReflectionException { return getInterceptorForInstance(name).getAttributes(name,attributes); } // From MBeanServer - public void setAttribute(ObjectName name, Attribute attribute) + public final void setAttribute(ObjectName name, Attribute attribute) throws InstanceNotFoundException, AttributeNotFoundException, InvalidAttributeValueException, MBeanException, ReflectionException { @@ -436,14 +438,14 @@ public abstract class DispatchInterceptor } // From MBeanServer - public AttributeList setAttributes(ObjectName name, + public final AttributeList setAttributes(ObjectName name, AttributeList attributes) throws InstanceNotFoundException, ReflectionException { return getInterceptorForInstance(name).setAttributes(name,attributes); } // From MBeanServer - public Object invoke(ObjectName name, String operationName, + public final Object invoke(ObjectName name, String operationName, Object params[], String signature[]) throws InstanceNotFoundException, MBeanException, ReflectionException { @@ -463,63 +465,69 @@ public abstract class DispatchInterceptor public abstract String[] getDomains(); // From MBeanServer - public void addNotificationListener(ObjectName name, + public final void addNotificationListener(ObjectName name, NotificationListener listener, NotificationFilter filter, Object handback) throws InstanceNotFoundException { - getInterceptorForInstance(name).addNotificationListener(name,listener,filter, + getInterceptorForInstance(name). + addNotificationListener(name,listener,filter, handback); } // From MBeanServer - public void addNotificationListener(ObjectName name, + public final void addNotificationListener(ObjectName name, ObjectName listener, NotificationFilter filter, Object handback) throws InstanceNotFoundException { - getInterceptorForInstance(name).addNotificationListener(name,listener,filter, + getInterceptorForInstance(name). + addNotificationListener(name,listener,filter, handback); } // From MBeanServer - public void removeNotificationListener(ObjectName name, + public final void removeNotificationListener(ObjectName name, ObjectName listener) throws InstanceNotFoundException, ListenerNotFoundException { - getInterceptorForInstance(name).removeNotificationListener(name,listener); + getInterceptorForInstance(name). + removeNotificationListener(name,listener); } // From MBeanServer - public void removeNotificationListener(ObjectName name, + public final void removeNotificationListener(ObjectName name, ObjectName listener, NotificationFilter filter, Object handback) throws InstanceNotFoundException, ListenerNotFoundException { - getInterceptorForInstance(name).removeNotificationListener(name,listener,filter, + getInterceptorForInstance(name). + removeNotificationListener(name,listener,filter, handback); } // From MBeanServer - public void removeNotificationListener(ObjectName name, + public final void removeNotificationListener(ObjectName name, NotificationListener listener) throws InstanceNotFoundException, ListenerNotFoundException { - getInterceptorForInstance(name).removeNotificationListener(name,listener); + getInterceptorForInstance(name). + removeNotificationListener(name,listener); } // From MBeanServer - public void removeNotificationListener(ObjectName name, + public final void removeNotificationListener(ObjectName name, NotificationListener listener, NotificationFilter filter, Object handback) throws InstanceNotFoundException, ListenerNotFoundException { - getInterceptorForInstance(name).removeNotificationListener(name,listener,filter, + getInterceptorForInstance(name). + removeNotificationListener(name,listener,filter, handback); } // From MBeanServer - public MBeanInfo getMBeanInfo(ObjectName name) + public final MBeanInfo getMBeanInfo(ObjectName name) throws InstanceNotFoundException, IntrospectionException, ReflectionException { return getInterceptorForInstance(name).getMBeanInfo(name); @@ -527,21 +535,23 @@ public abstract class DispatchInterceptor // From MBeanServer - public boolean isInstanceOf(ObjectName name, String className) + public final boolean isInstanceOf(ObjectName name, String className) throws InstanceNotFoundException { return getInterceptorForInstance(name).isInstanceOf(name,className); } // From MBeanServer - public ClassLoader getClassLoaderFor(ObjectName mbeanName) + public final ClassLoader getClassLoaderFor(ObjectName mbeanName) throws InstanceNotFoundException { - return getInterceptorForInstance(mbeanName).getClassLoaderFor(mbeanName); + return getInterceptorForInstance(mbeanName). + getClassLoaderFor(mbeanName); } // From MBeanServer - public ClassLoader getClassLoader(ObjectName loaderName) + public final ClassLoader getClassLoader(ObjectName loaderName) throws InstanceNotFoundException { - return getInterceptorForInstance(loaderName).getClassLoader(loaderName); + return getInterceptorForInstance(loaderName). + getClassLoader(loaderName); } } diff --git a/src/share/classes/com/sun/jmx/interceptor/DomainDispatchInterceptor.java b/src/share/classes/com/sun/jmx/interceptor/DomainDispatchInterceptor.java index cb1489ee5..9b9b1d6b1 100644 --- a/src/share/classes/com/sun/jmx/interceptor/DomainDispatchInterceptor.java +++ b/src/share/classes/com/sun/jmx/interceptor/DomainDispatchInterceptor.java @@ -75,7 +75,7 @@ class DomainDispatchInterceptor private final DomainDispatchInterceptor parent; AggregatingQueryInterceptor(DomainDispatchInterceptor dispatcher) { - super(dispatcher.localNamespace); + super(dispatcher.nextInterceptor); parent = dispatcher; } @@ -91,9 +91,8 @@ class DomainDispatchInterceptor // Add all matching MBeans from local namespace. final Set res = Util.cloneSet(local); - final boolean all = (pattern == null || - pattern.getDomain().equals("*")); if (pattern == null) pattern = ObjectName.WILDCARD; + final boolean all = pattern.getDomain().equals("*"); final String domain = pattern.getDomain(); @@ -142,7 +141,7 @@ class DomainDispatchInterceptor } } - private final DefaultMBeanServerInterceptor localNamespace; + private final DefaultMBeanServerInterceptor nextInterceptor; private final String mbeanServerName; private final MBeanServerDelegate delegate; @@ -165,7 +164,7 @@ class DomainDispatchInterceptor MBeanInstantiator instantiator, Repository repository, NamespaceDispatchInterceptor namespaces) { - localNamespace = new DefaultMBeanServerInterceptor(outer, + nextInterceptor = new DefaultMBeanServerInterceptor(outer, delegate, instantiator,repository,namespaces); mbeanServerName = Util.getMBeanServerSecurityName(delegate); this.delegate = delegate; @@ -182,7 +181,7 @@ class DomainDispatchInterceptor @Override void validateHandlerNameFor(String key, ObjectName name) { super.validateHandlerNameFor(key,name); - final String[] domains = localNamespace.getDomains(); + final String[] domains = nextInterceptor.getDomains(); for (int i=0;i postRegisterQueue) { if (handler instanceof JMXDomain) - localNamespace.addNamespace(name, + nextInterceptor.addInterceptorFor(name, (JMXDomain)handler,postRegisterQueue); - else super.addNamespace(name,handler,postRegisterQueue); + else super.addInterceptorFor(name,handler,postRegisterQueue); } @Override - public void removeNamespace(ObjectName name, JMXNamespace handler, + public void removeInterceptorFor(ObjectName name, JMXNamespace handler, Queue postDeregisterQueue) { if (handler instanceof JMXDomain) - localNamespace.removeNamespace(name,(JMXDomain)handler, + nextInterceptor.removeInterceptorFor(name,(JMXDomain)handler, postDeregisterQueue); - else super.removeNamespace(name,handler,postDeregisterQueue); + else super.removeInterceptorFor(name,handler,postDeregisterQueue); } diff --git a/src/share/classes/com/sun/jmx/mbeanserver/Util.java b/src/share/classes/com/sun/jmx/mbeanserver/Util.java index fcbdd9041..93c5c97f6 100644 --- a/src/share/classes/com/sun/jmx/mbeanserver/Util.java +++ b/src/share/classes/com/sun/jmx/mbeanserver/Util.java @@ -57,7 +57,8 @@ import static javax.management.namespace.JMXNamespaces.NAMESPACE_SEPARATOR; public class Util { private final static int NAMESPACE_SEPARATOR_LENGTH = NAMESPACE_SEPARATOR.length(); - public final static String ILLEGAL_MBEANSERVER_NAME_CHARS=";:*?"; + public final static char[] ILLEGAL_MBEANSERVER_NAME_CHARS=";:*?". + toCharArray(); static Map newMap() { @@ -621,7 +622,7 @@ public class Util { * is {@code null}. * @throws IllegalArgumentException if mbeanServerName contains illegal * characters, or is empty, or is {@code "-"}. - * Illegal characters are {@value #ILLEGAL_MBEANSERVER_NAME_CHARS}. + * Illegal characters are {@link #ILLEGAL_MBEANSERVER_NAME_CHARS}. */ public static String checkServerName(String mbeanServerName) { if ("".equals(mbeanServerName)) @@ -632,7 +633,7 @@ public class Util { "\"-\" is not a valid MBean server name"); if (isMBeanServerNameUndefined(mbeanServerName)) return MBeanServerFactory.DEFAULT_MBEANSERVER_NAME; - for (char c : ILLEGAL_MBEANSERVER_NAME_CHARS.toCharArray()) { + for (char c : ILLEGAL_MBEANSERVER_NAME_CHARS) { if (mbeanServerName.indexOf(c) >= 0) throw new IllegalArgumentException( "invalid character in MBeanServer name: "+c); @@ -662,15 +663,15 @@ public class Util { } // Log the exception and its causes without logging the stack trace. - // Use with care - it is usally preferable to log the whole stack trace! + // Use with care - it is usually preferable to log the whole stack trace! // We don't want to log the whole stack trace here: logshort() is // called in those cases where the exception might not be abnormal. private static void logshort(String msg, Throwable t) { if (JmxProperties.MISC_LOGGER.isLoggable(Level.FINE)) { StringBuilder toprint = new StringBuilder(msg); - toprint.append("\nCaused By: ").append(String.valueOf(t)); - while ((t=t.getCause())!=null) - toprint.append("\nCaused By: ").append(String.valueOf(t)); + do { + toprint.append("\nCaused By: ").append(String.valueOf(t)); + } while ((t=t.getCause())!=null); JmxProperties.MISC_LOGGER.fine(toprint.toString()); } } diff --git a/src/share/classes/com/sun/jmx/namespace/DomainInterceptor.java b/src/share/classes/com/sun/jmx/namespace/DomainInterceptor.java index d7619814d..84644d831 100644 --- a/src/share/classes/com/sun/jmx/namespace/DomainInterceptor.java +++ b/src/share/classes/com/sun/jmx/namespace/DomainInterceptor.java @@ -85,7 +85,7 @@ public class DomainInterceptor extends HandlerInterceptor { final ObjectName pattern; public PatternNotificationFilter(ObjectName pattern) { - this.pattern = pattern; + this.pattern = pattern==null?ObjectName.WILDCARD:pattern; } public boolean isNotificationEnabled(Notification notification) { @@ -93,7 +93,7 @@ public class DomainInterceptor extends HandlerInterceptor { return false; final MBeanServerNotification mbsn = (MBeanServerNotification) notification; - if (pattern == null || pattern.apply(mbsn.getMBeanName())) + if (pattern.apply(mbsn.getMBeanName())) return true; return false; } @@ -110,6 +110,7 @@ public class DomainInterceptor extends HandlerInterceptor { super(handler); this.domainName = domainName; this.serverName = serverName; + ALL = Util.newObjectName(domainName+":*"); } @Override @@ -118,27 +119,27 @@ public class DomainInterceptor extends HandlerInterceptor { ", domain="+this.domainName+")"; } - public void connectDelegate(final MBeanServerDelegate delegate) + final void connectDelegate(final MBeanServerDelegate delegate) throws InstanceNotFoundException { final NotificationFilter filter = new PatternNotificationFilter(getPatternFor(null)); synchronized (this) { - if (mbsListener == null) + if (mbsListener == null) { mbsListener = new NotificationListener() { - - public void handleNotification(Notification notification, - Object handback) { - if (filter.isNotificationEnabled(notification)) - delegate.sendNotification(notification); - } - }; + public void handleNotification(Notification notification, + Object handback) { + if (filter.isNotificationEnabled(notification)) + delegate.sendNotification(notification); + } + }; + } } - getNamespace(). + getHandlerInterceptorMBean(). addMBeanServerNotificationListener(mbsListener, filter); } - public void disconnectDelegate() + final void disconnectDelegate() throws InstanceNotFoundException, ListenerNotFoundException { final NotificationListener l; synchronized (this) { @@ -146,10 +147,10 @@ public class DomainInterceptor extends HandlerInterceptor { if (l == null) return; mbsListener = null; } - getNamespace().removeMBeanServerNotificationListener(l); + getHandlerInterceptorMBean().removeMBeanServerNotificationListener(l); } - public void addPostRegisterTask(Queue queue, + public final void addPostRegisterTask(Queue queue, final MBeanServerDelegate delegate) { if (queue == null) throw new IllegalArgumentException("task queue must not be null"); @@ -158,14 +159,15 @@ public class DomainInterceptor extends HandlerInterceptor { try { connectDelegate(delegate); } catch (Exception x) { - throw new UnsupportedOperationException("notification forwarding",x); + throw new UnsupportedOperationException( + "notification forwarding",x); } } }; queue.add(task1); } - public void addPostDeregisterTask(Queue queue, + public final void addPostDeregisterTask(Queue queue, final MBeanServerDelegate delegate) { if (queue == null) throw new IllegalArgumentException("task queue must not be null"); @@ -174,17 +176,18 @@ public class DomainInterceptor extends HandlerInterceptor { try { disconnectDelegate(); } catch (Exception x) { - throw new UnsupportedOperationException("notification forwarding",x); + throw new UnsupportedOperationException( + "notification forwarding",x); } } }; queue.add(task1); } - /** - * Throws IllegalArgumentException if targetName.getDomain() is not - * in the domain handled. - **/ + // No name conversion for JMXDomains... + // Throws IllegalArgumentException if targetName.getDomain() is not + // in the domain handled. + // @Override protected ObjectName toSource(ObjectName targetName) { if (targetName == null) return null; @@ -198,6 +201,7 @@ public class DomainInterceptor extends HandlerInterceptor { return targetName; } + // No name conversion for JMXDomains... @Override protected ObjectName toTarget(ObjectName sourceName) { return sourceName; @@ -255,16 +259,16 @@ public class DomainInterceptor extends HandlerInterceptor { if (LOG.isLoggable(Level.FINE)) LOG.fine("Unexpected exception raised in queryNames: "+x); LOG.log(Level.FINEST,"Unexpected exception raised in queryNames",x); + return Collections.emptySet(); } - // We reach here only when an exception was raised. - // - final Set empty = Collections.emptySet(); - return empty; } + // Compute a new pattern which is a sub pattern of 'name' but only selects + // the MBeans in domain 'domainName' + // When we reach here, it has been verified that 'name' matches our domain + // name (done by DomainDispatchInterceptor) private ObjectName getPatternFor(final ObjectName name) { try { - if (ALL == null) ALL = ObjectName.getInstance(domainName + ":*"); if (name == null) return ALL; if (name.getDomain().equals(domainName)) return name; return name.withDomain(domainName); @@ -284,11 +288,8 @@ public class DomainInterceptor extends HandlerInterceptor { if (LOG.isLoggable(Level.FINE)) LOG.fine("Unexpected exception raised in queryNames: "+x); LOG.log(Level.FINEST,"Unexpected exception raised in queryNames",x); + return Collections.emptySet(); } - // We reach here only when an exception was raised. - // - final Set empty = Collections.emptySet(); - return empty; } @Override @@ -306,7 +307,7 @@ public class DomainInterceptor extends HandlerInterceptor { // in the domain. @Override public Integer getMBeanCount() { - return getNamespace().getMBeanCount(); + return getHandlerInterceptorMBean().getMBeanCount(); } private boolean checkOn() { @@ -320,8 +321,8 @@ public class DomainInterceptor extends HandlerInterceptor { @Override void check(ObjectName routingName, String member, String action) { if (!checkOn()) return; - final String act = (action==null)?"-":action.intern(); - if(act == "queryMBeans" || act == "queryNames") { // ES: OK + final String act = (action==null)?"-":action; + if("queryMBeans".equals(act) || "queryNames".equals(act)) { // This is tricky. check with 3 parameters is called // by queryNames/queryMBeans before performing the query. // At this point we must check with no class name. @@ -355,16 +356,8 @@ public class DomainInterceptor extends HandlerInterceptor { if (!checkOn()) return; final MBeanPermission perm; - // action is most probably already an intern string. - // string literals are intern strings. - // we create a new intern string for 'action' - just to be on - // the safe side... - // We intern it in order to be able to use == rather than equals - // below, because if we don't, and if action is not one of the - // 4 literals below, we would have to do a full string comparison. - // - final String act = (action==null)?"-":action.intern(); - if (act == "getDomains") { // ES: OK + final String act = (action==null)?"-":action; + if ("getDomains".equals(act)) { // ES: OK perm = new MBeanPermission(serverName,"-",member, routingName,act); } else { @@ -381,7 +374,7 @@ public class DomainInterceptor extends HandlerInterceptor { String getClassName(ObjectName routingName) { if (routingName == null || routingName.isPattern()) return "-"; try { - return getNamespace().getSourceServer(). + return getHandlerInterceptorMBean().getSourceServer(). getObjectInstance(routingName).getClassName(); } catch (InstanceNotFoundException ex) { LOG.finest("Can't get class name for "+routingName+ diff --git a/src/share/classes/com/sun/jmx/namespace/HandlerInterceptor.java b/src/share/classes/com/sun/jmx/namespace/HandlerInterceptor.java index eb48ef849..7c2f39348 100644 --- a/src/share/classes/com/sun/jmx/namespace/HandlerInterceptor.java +++ b/src/share/classes/com/sun/jmx/namespace/HandlerInterceptor.java @@ -63,8 +63,8 @@ import javax.management.namespace.JMXNamespace; /** * This interceptor wraps a JMXNamespace, and performs * {@code ObjectName} rewriting. {@code HandlerInterceptor} are - * usually created and managed by a {@link NamespaceDispatcher} or - * {@link DomainDispatcher}. + * created and managed by a {@link NamespaceDispatchInterceptor} or a + * {@link DomainDispatchInterceptor}. *

* This API is a Sun internal API and is subject to changes without notice. *

@@ -90,6 +90,12 @@ public abstract class HandlerInterceptor this.handler = handler; } + // + // The {@code source} connection is a connection to the MBeanServer + // that contains the actual MBeans. + // In the case of cascading, that would be a connection to the sub + // agent. Practically, this is JMXNamespace.getSourceServer(); + // @Override protected MBeanServer source() { return handler.getSourceServer(); @@ -105,7 +111,9 @@ public abstract class HandlerInterceptor return source(); } - T getNamespace() { + // The namespace or domain handler - this either a JMXNamespace or a + // a JMXDomain + T getHandlerInterceptorMBean() { return handler; } @@ -122,7 +130,7 @@ public abstract class HandlerInterceptor Util.newRuntimeIOException(x)); } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public AttributeList getAttributes(ObjectName name, String[] attributes) throws InstanceNotFoundException, ReflectionException { @@ -172,7 +180,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public void removeNotificationListener(ObjectName name, ObjectName listener) throws InstanceNotFoundException, ListenerNotFoundException { @@ -183,7 +191,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public String getDefaultDomain() { try { @@ -193,7 +201,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public String[] getDomains() { try { @@ -203,7 +211,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public Integer getMBeanCount() { try { @@ -213,7 +221,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public void setAttribute(ObjectName name, Attribute attribute) throws InstanceNotFoundException, AttributeNotFoundException, @@ -226,7 +234,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public Set queryNames(ObjectName name, QueryExp query) { try { @@ -236,7 +244,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public Set queryMBeans(ObjectName name, QueryExp query) { try { @@ -246,7 +254,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public boolean isInstanceOf(ObjectName name, String className) throws InstanceNotFoundException { @@ -257,7 +265,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public ObjectInstance createMBean(String className, ObjectName name) throws ReflectionException, InstanceAlreadyExistsException, @@ -270,7 +278,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public ObjectInstance createMBean(String className, ObjectName name, ObjectName loaderName) @@ -284,7 +292,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public Object getAttribute(ObjectName name, String attribute) throws MBeanException, AttributeNotFoundException, @@ -296,7 +304,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public void removeNotificationListener(ObjectName name, ObjectName listener, NotificationFilter filter, Object handback) @@ -309,7 +317,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public void removeNotificationListener(ObjectName name, NotificationListener listener, NotificationFilter filter, @@ -323,7 +331,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public void removeNotificationListener(ObjectName name, NotificationListener listener) @@ -336,7 +344,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public void addNotificationListener(ObjectName name, NotificationListener listener, NotificationFilter filter, @@ -349,7 +357,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public void addNotificationListener(ObjectName name, ObjectName listener, NotificationFilter filter, Object handback) @@ -362,7 +370,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public boolean isRegistered(ObjectName name) { try { @@ -372,7 +380,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public void unregisterMBean(ObjectName name) throws InstanceNotFoundException, MBeanRegistrationException { @@ -383,7 +391,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public MBeanInfo getMBeanInfo(ObjectName name) throws InstanceNotFoundException, IntrospectionException, @@ -395,7 +403,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public ObjectInstance getObjectInstance(ObjectName name) throws InstanceNotFoundException { @@ -406,7 +414,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public ObjectInstance createMBean(String className, ObjectName name, Object[] params, String[] signature) @@ -421,7 +429,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public ObjectInstance createMBean(String className, ObjectName name, ObjectName loaderName, Object[] params, String[] signature) @@ -437,7 +445,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public AttributeList setAttributes(ObjectName name,AttributeList attributes) throws InstanceNotFoundException, ReflectionException { @@ -448,7 +456,7 @@ public abstract class HandlerInterceptor } } - // From MBeanServer: catch & handles IOException + // From MBeanServerConnection: catch & handles IOException @Override public Object invoke(ObjectName name, String operationName, Object[] params, String[] signature) diff --git a/src/share/classes/com/sun/jmx/namespace/JMXNamespaceUtils.java b/src/share/classes/com/sun/jmx/namespace/JMXNamespaceUtils.java index 1fccfcbc7..49f875144 100644 --- a/src/share/classes/com/sun/jmx/namespace/JMXNamespaceUtils.java +++ b/src/share/classes/com/sun/jmx/namespace/JMXNamespaceUtils.java @@ -29,7 +29,6 @@ import com.sun.jmx.defaults.JmxProperties; import java.io.IOException; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import java.util.WeakHashMap; import java.util.logging.Level; @@ -40,6 +39,8 @@ import javax.management.MBeanServerConnection; import javax.management.NotificationFilter; import javax.management.NotificationListener; import javax.management.event.EventClient; +import javax.management.event.EventClientDelegateMBean; +import javax.management.namespace.JMXNamespace; import javax.management.namespace.JMXNamespaces; import javax.management.remote.JMXAddressable; import javax.management.remote.JMXConnector; @@ -66,26 +67,10 @@ public final class JMXNamespaceUtils { return new WeakHashMap(); } - /** Creates a new instance of JMXNamespaces */ + /** There are no instances of this class */ private JMXNamespaceUtils() { } - /** - * Returns an unmodifiable option map in which the given keys have been - * filtered out. - * @param keys keys to filter out from the map. - * @return An unmodifiable option map in which the given keys have been - * filtered out. - */ - public static Map filterMap(Map map, K... keys) { - final Map filtered; - filtered=new HashMap(map); - for (K key : keys) { - filtered.remove(key); - } - return unmodifiableMap(filtered); - } - // returns un unmodifiable view of a map. public static Map unmodifiableMap(Map aMap) { if (aMap == null || aMap.isEmpty()) diff --git a/src/share/classes/com/sun/jmx/namespace/NamespaceInterceptor.java b/src/share/classes/com/sun/jmx/namespace/NamespaceInterceptor.java index da85fc6d4..11afeb2af 100644 --- a/src/share/classes/com/sun/jmx/namespace/NamespaceInterceptor.java +++ b/src/share/classes/com/sun/jmx/namespace/NamespaceInterceptor.java @@ -54,10 +54,6 @@ import javax.management.namespace.JMXNamespacePermission; */ public class NamespaceInterceptor extends HandlerInterceptor { - /** - * A logger for this class. - **/ - private static final Logger LOG = JmxProperties.NAMESPACE_LOGGER; private static final Logger PROBE_LOG = Logger.getLogger( JmxProperties.NAMESPACE_LOGGER+".probe"); diff --git a/src/share/classes/com/sun/jmx/namespace/RoutingConnectionProxy.java b/src/share/classes/com/sun/jmx/namespace/RoutingConnectionProxy.java index 787343eec..443c80f2a 100644 --- a/src/share/classes/com/sun/jmx/namespace/RoutingConnectionProxy.java +++ b/src/share/classes/com/sun/jmx/namespace/RoutingConnectionProxy.java @@ -45,6 +45,9 @@ import javax.management.namespace.JMXNamespaces; *

* @since 1.7 */ +// See class hierarchy and detailled explanations in RoutingProxy in this +// package. +// public class RoutingConnectionProxy extends RoutingProxy { @@ -93,40 +96,28 @@ public class RoutingConnectionProxy targetNs+"\", "+forwardsContext+")"; } + static final RoutingProxyFactory + + FACTORY = new RoutingProxyFactory + () { + + public RoutingConnectionProxy newInstance(MBeanServerConnection source, + String sourcePath, String targetPath, + boolean forwardsContext) { + return new RoutingConnectionProxy(source,sourcePath, + targetPath,forwardsContext); + } + + public RoutingConnectionProxy newInstance( + MBeanServerConnection source, String sourcePath) { + return new RoutingConnectionProxy(source,sourcePath); + } + }; + public static MBeanServerConnection cd(MBeanServerConnection source, String sourcePath) { - if (source == null) throw new IllegalArgumentException("null"); - if (source.getClass().equals(RoutingConnectionProxy.class)) { - // cast is OK here, but findbugs complains unless we use class.cast - final RoutingConnectionProxy other = - RoutingConnectionProxy.class.cast(source); - final String target = other.getTargetNamespace(); - - // Avoid multiple layers of serialization. - // - // We construct a new proxy from the original source instead of - // stacking a new proxy on top of the old one. - // - that is we replace - // cd ( cd ( x, dir1), dir2); - // by - // cd (x, dir1//dir2); - // - // We can do this only when the source class is exactly - // NamespaceConnectionProxy. - // - if (target == null || target.equals("")) { - final String path = - JMXNamespaces.concat(other.getSourceNamespace(), - sourcePath); - return new RoutingConnectionProxy(other.source(),path,"", - other.forwardsContext); - } - // Note: we could do possibly something here - but it would involve - // removing part of targetDir, and possibly adding - // something to sourcePath. - // Too complex to bother! => simply default to stacking... - } - return new RoutingConnectionProxy(source,sourcePath); + return RoutingProxy.cd(RoutingConnectionProxy.class, FACTORY, + source, sourcePath); } } diff --git a/src/share/classes/com/sun/jmx/namespace/RoutingMBeanServerConnection.java b/src/share/classes/com/sun/jmx/namespace/RoutingMBeanServerConnection.java index 904e5848e..70df9b504 100644 --- a/src/share/classes/com/sun/jmx/namespace/RoutingMBeanServerConnection.java +++ b/src/share/classes/com/sun/jmx/namespace/RoutingMBeanServerConnection.java @@ -83,18 +83,32 @@ public abstract class RoutingMBeanServerConnection//}. - * @param attributes The list of attributes to check permission for. - * @param action one of "getAttribute" or "setAttribute" - * @return The list of attributes for which the callers has the - * appropriate {@link - * javax.management.namespace.JMXNamespacePermission}. - */ - String[] checkAttributes(ObjectName routingName, - String[] attributes, String action) { - check(routingName,null,action); - return attributes; - } - - /** - * This method is a hook to implement permission checking in subclasses. - * By default, this method does nothing and simply returns - * {@code attribute}. - * - * @param routingName The name of the MBean in the enclosing context. - * This is of the form {@code //}. - * @param attributes The list of attributes to check permission for. - * @param action one of "getAttribute" or "setAttribute" - * @return The list of attributes for which the callers has the - * appropriate {@link - * javax.management.namespace.JMXNamespacePermission}. - */ - AttributeList checkAttributes(ObjectName routingName, - AttributeList attributes, String action) { - check(routingName,null,action); - return attributes; - } - // from MBeanServerConnection public AttributeList getAttributes(ObjectName name, String[] attributes) throws InstanceNotFoundException, ReflectionException, IOException { @@ -195,37 +171,6 @@ public abstract class RoutingMBeanServerConnection//}. - * @param member The {@link - * javax.management.namespace.JMXNamespacePermission#getMember member} - * name. - * @param action The {@link - * javax.management.namespace.JMXNamespacePermission#getActions action} - * name. - */ - void check(ObjectName routingName, - String member, String action) { - } - - void checkPattern(ObjectName routingPattern, - String member, String action) { - // pattern is checked only at posteriori by checkQuery. - // checking it a priori usually doesn't work, because ObjectName.apply - // does not work between two patterns. - check(null,null,action); - } - - void checkCreate(ObjectName routingName, String className, - String action) { - } - // from MBeanServerConnection public Object invoke(ObjectName name, String operationName, Object[] params, String[] signature) @@ -446,24 +391,6 @@ public abstract class RoutingMBeanServerConnection//}. - * @param action one of "queryNames" or "queryMBeans" - * @return true if {@code sourceName} can be returned. - */ - boolean checkQuery(ObjectName routingName, String action) { - return true; - } // Return names in the target's context. ObjectInstance processOutputInstance(ObjectInstance source) { @@ -643,6 +570,111 @@ public abstract class RoutingMBeanServerConnection//}. + * @param attributes The list of attributes to check permission for. + * @param action one of "getAttribute" or "setAttribute" + * @return The list of attributes for which the callers has the + * appropriate {@link + * javax.management.namespace.JMXNamespacePermission}. + */ + String[] checkAttributes(ObjectName routingName, + String[] attributes, String action) { + check(routingName,null,action); + return attributes; + } + + /** + * This method is a hook to implement permission checking in subclasses. + * By default, this method does nothing and simply returns + * {@code attribute}. + * + * @param routingName The name of the MBean in the enclosing context. + * This is of the form {@code //}. + * @param attributes The list of attributes to check permission for. + * @param action one of "getAttribute" or "setAttribute" + * @return The list of attributes for which the callers has the + * appropriate {@link + * javax.management.namespace.JMXNamespacePermission}. + */ + AttributeList checkAttributes(ObjectName routingName, + AttributeList attributes, String action) { + check(routingName,null,action); + return attributes; + } + + /** + * This method is a hook to implement permission checking in subclasses. + * By default, this method does nothing. + * A subclass may override this method and throw a {@link + * SecurityException} if the permission is denied. + * + * @param routingName The name of the MBean in the enclosing context. + * This is of the form {@code //}. + * @param member The {@link + * javax.management.namespace.JMXNamespacePermission#getMember member} + * name. + * @param action The {@link + * javax.management.namespace.JMXNamespacePermission#getActions action} + * name. + */ + void check(ObjectName routingName, + String member, String action) { + } + + // called in createMBean and registerMBean + void checkCreate(ObjectName routingName, String className, + String action) { + } + + // A priori check for queryNames/queryMBeans/ + void checkPattern(ObjectName routingPattern, + String member, String action) { + // pattern is checked only at posteriori by checkQuery. + // checking it a priori usually doesn't work, because ObjectName.apply + // does not work between two patterns. + // We only check that we have the permission requested for 'action'. + check(null,null,action); + } + + + /** + * This is a hook to implement permission checking in subclasses. + * + * Checks that the caller has sufficient permission for returning + * information about {@code sourceName} in {@code action}. + * + * By default always return true. Subclass may override this method + * and return false if the caller doesn't have sufficient permissions. + * + * @param routingName The name of the MBean to include or exclude from + * the query, expressed in the enclosing context. + * This is of the form {@code //}. + * @param action one of "queryNames" or "queryMBeans" + * @return true if {@code sourceName} can be returned. + */ + boolean checkQuery(ObjectName routingName, String action) { + return true; + } + /** * This method is a hook to implement permission checking in subclasses. * Checks that the caller as the necessary permissions to view the @@ -658,14 +690,4 @@ public abstract class RoutingMBeanServerConnection{@link RoutingConnectionProxy}: to cd in an MBeanServerConnection.

- *

{@link RoutingServerProxy}: to cd in an MBeanServer.

+ *

{@link RoutingConnectionProxy}: to narrow down into an + * MBeanServerConnection.

+ *

{@link RoutingServerProxy}: to narrow down into an MBeanServer.

*

* This API is a Sun internal API and is subject to changes without notice. *

* @since 1.7 */ +// +// RoutingProxies are client side objects which are used to narrow down +// into a namespace. They are used to perform ObjectName translation, +// adding the namespace to the routing ObjectName before sending it over +// to the source connection, and removing that prefix from results of +// queries, createMBean, registerMBean, and getObjectInstance. +// This translation is the opposite to that which is performed by +// NamespaceInterceptors. +// +// There is however a special case where routing proxies are used on the +// 'server' side to remove a namespace - rather than to add it: +// This the case of ClientContext. +// When an ObjectName like "jmx.context//c1=v1,c2=v2//D:k=v" reaches the +// jmx.context namespace, a routing proxy is used to remove the prefix +// c1=v1,c2=v2// from the routing objectname. +// +// For a RoutingProxy used in a narrowDownToNamespace operation, we have: +// targetNs="" // targetNS is the namespace 'to remove' +// sourceNS= // namespace 'to add' +// +// For a RoutingProxy used in a ClientContext operation, we have: +// targetNs= // context must be removed from object name +// sourceNs="" // nothing to add... +// +// RoutingProxies can also be used on the client side to implement +// "withClientContext" operations. In that case, the boolean parameter +// 'forwards context' is set to true, targetNs is "", and sourceNS may +// also be "". When forwardsContext is true, the RoutingProxy dynamically +// creates an ObjectNameRouter for each operation - in order to dynamically add +// the context attached to the thread to the routing ObjectName. This is +// performed in the getObjectNameRouter() method. +// +// Finally, in order to avoid too many layers of wrapping, +// RoutingConnectionProxy and RoutingServerProxy can be created through a +// factory method that can concatenate namespace pathes in order to +// return a single RoutingProxy - rather than wrapping a RoutingProxy inside +// another RoutingProxy. See RoutingConnectionProxy.cd and +// RoutingServerProxy.cd +// +// The class hierarchy is as follows: +// +// RoutingMBeanServerConnection +// [abstract class for all routing interceptors, +// such as RoutingProxies and HandlerInterceptors] +// / \ +// / \ +// RoutingProxy HandlerInterceptor +// [base class for [base class for server side +// client-side objects used objects, created by +// in narrowDownTo] DispatchInterceptors] +// / \ | \ +// RoutingConnectionProxy \ | NamespaceInterceptor +// [wraps MBeanServerConnection \ | [used to remove +// objects] \ | namespace prefix and +// RoutingServerProxy | wrap JMXNamespace] +// [wraps MBeanServer | +// Objects] | +// DomainInterceptor +// [used to wrap JMXDomain] +// +// RoutingProxies also differ from HandlerInterceptors in that they transform +// calls to MBeanServerConnection operations that do not have any parameters +// into a call to the underlying JMXNamespace MBean. +// So for instance a call to: +// JMXNamespaces.narrowDownToNamespace(conn,"foo").getDomains() +// is transformed into +// conn.getAttribute("foo//type=JMXNamespace","Domains"); +// public abstract class RoutingProxy extends RoutingMBeanServerConnection { @@ -179,17 +245,11 @@ public abstract class RoutingProxy throw x; } catch (MBeanException ex) { throw new IOException("Failed to get "+attributeName+": "+ - ex.getMessage(), + ex, ex.getTargetException()); - } catch (AttributeNotFoundException ex) { + } catch (Exception ex) { throw new IOException("Failed to get "+attributeName+": "+ - ex.getMessage(),ex); - } catch (InstanceNotFoundException ex) { - throw new IOException("Failed to get "+attributeName+": "+ - ex.getMessage(),ex); - } catch (ReflectionException ex) { - throw new IOException("Failed to get "+attributeName+": "+ - ex.getMessage(),ex); + ex,ex); } } @@ -279,4 +339,62 @@ public abstract class RoutingProxy (" mounted on targetNs="+targetNs)); } + // Creates an instance of a subclass 'R' of RoutingProxy + // RoutingServerProxy and RoutingConnectionProxy have their own factory + // instance. + static interface RoutingProxyFactory> { + R newInstance(T source, + String sourcePath, String targetPath, + boolean forwardsContext); + R newInstance(T source, + String sourcePath); + } + + // Performs a narrowDownToNamespace operation. + // This method will attempt to merge two RoutingProxies in a single + // one if they are of the same class. + // + // This method is never called directly - it should be called only by + // subclasses of RoutingProxy. + // + // As for now it is called by: + // RoutingServerProxy.cd and RoutingConnectionProxy.cd. + // + static > + R cd(Class routingProxyClass, + RoutingProxyFactory factory, + T source, String sourcePath) { + if (source == null) throw new IllegalArgumentException("null"); + if (source.getClass().equals(routingProxyClass)) { + // cast is OK here, but findbugs complains unless we use class.cast + final R other = routingProxyClass.cast(source); + final String target = other.getTargetNamespace(); + + // Avoid multiple layers of serialization. + // + // We construct a new proxy from the original source instead of + // stacking a new proxy on top of the old one. + // - that is we replace + // cd ( cd ( x, dir1), dir2); + // by + // cd (x, dir1//dir2); + // + // We can do this only when the source class is exactly + // RoutingServerProxy. + // + if (target == null || target.equals("")) { + final String path = + JMXNamespaces.concat(other.getSourceNamespace(), + sourcePath); + return factory.newInstance(other.source(),path,"", + other.forwardsContext); + } + // Note: we could do possibly something here - but it would involve + // removing part of targetDir, and possibly adding + // something to sourcePath. + // Too complex to bother! => simply default to stacking... + } + return factory.newInstance(source,sourcePath); + } } diff --git a/src/share/classes/com/sun/jmx/namespace/RoutingServerProxy.java b/src/share/classes/com/sun/jmx/namespace/RoutingServerProxy.java index 94aa139dd..f58e39816 100644 --- a/src/share/classes/com/sun/jmx/namespace/RoutingServerProxy.java +++ b/src/share/classes/com/sun/jmx/namespace/RoutingServerProxy.java @@ -69,6 +69,9 @@ import javax.management.namespace.JMXNamespaces; * * @since 1.7 */ +// See class hierarchy and detailled explanations in RoutingProxy in this +// package. +// public class RoutingServerProxy extends RoutingProxy implements MBeanServer { @@ -564,39 +567,24 @@ public class RoutingServerProxy } } + static final RoutingProxyFactory + FACTORY = new RoutingProxyFactory() { + + public RoutingServerProxy newInstance(MBeanServer source, + String sourcePath, String targetPath, + boolean forwardsContext) { + return new RoutingServerProxy(source,sourcePath, + targetPath,forwardsContext); + } + + public RoutingServerProxy newInstance( + MBeanServer source, String sourcePath) { + return new RoutingServerProxy(source,sourcePath); + } + }; public static MBeanServer cd(MBeanServer source, String sourcePath) { - if (source == null) throw new IllegalArgumentException("null"); - if (source.getClass().equals(RoutingServerProxy.class)) { - // cast is OK here, but findbugs complains unless we use class.cast - final RoutingServerProxy other = - RoutingServerProxy.class.cast(source); - final String target = other.getTargetNamespace(); - - // Avoid multiple layers of serialization. - // - // We construct a new proxy from the original source instead of - // stacking a new proxy on top of the old one. - // - that is we replace - // cd ( cd ( x, dir1), dir2); - // by - // cd (x, dir1//dir2); - // - // We can do this only when the source class is exactly - // NamespaceServerProxy. - // - if (target == null || target.equals("")) { - final String path = - JMXNamespaces.concat(other.getSourceNamespace(), - sourcePath); - return new RoutingServerProxy(other.source(),path,"", - other.forwardsContext); - } - // Note: we could do possibly something here - but it would involve - // removing part of targetDir, and possibly adding - // something to sourcePath. - // Too complex to bother! => simply default to stacking... - } - return new RoutingServerProxy(source,sourcePath); + return RoutingProxy.cd(RoutingServerProxy.class, FACTORY, + source, sourcePath); } } diff --git a/src/share/classes/javax/management/namespace/JMXDomain.java b/src/share/classes/javax/management/namespace/JMXDomain.java index a54fde7f3..bff3c1370 100644 --- a/src/share/classes/javax/management/namespace/JMXDomain.java +++ b/src/share/classes/javax/management/namespace/JMXDomain.java @@ -308,17 +308,17 @@ public class JMXDomain extends JMXNamespace { * It is however only available for subclasses in this package. **/ @Override - ObjectName validateHandlerName(ObjectName supliedName) { - if (supliedName == null) + ObjectName validateHandlerName(ObjectName suppliedName) { + if (suppliedName == null) throw new IllegalArgumentException("Must supply a valid name"); final String dirName = JMXNamespaces. - normalizeNamespaceName(supliedName.getDomain()); + normalizeNamespaceName(suppliedName.getDomain()); final ObjectName handlerName = getDomainObjectName(dirName); - if (!supliedName.equals(handlerName)) + if (!suppliedName.equals(handlerName)) throw new IllegalArgumentException("invalid name space name: "+ - supliedName); + suppliedName); - return supliedName; + return suppliedName; } /** diff --git a/src/share/classes/javax/management/namespace/JMXNamespace.java b/src/share/classes/javax/management/namespace/JMXNamespace.java index 39cb11b02..23f3004eb 100644 --- a/src/share/classes/javax/management/namespace/JMXNamespace.java +++ b/src/share/classes/javax/management/namespace/JMXNamespace.java @@ -482,8 +482,8 @@ public class JMXNamespace /** * This method is part of the {@link MBeanRegistration} interface. * The {@link JMXNamespace} class uses the {@link MBeanRegistration} - * interface in order to get a handle to the MBean server in which it is - * registered. It also check the validity of its own ObjectName. + * interface in order to get a reference to the MBean server in which it is + * registered. It also checks the validity of its own ObjectName. *

* This method is called by the MBean server. * Application classes should never call this method directly. @@ -502,11 +502,14 @@ public class JMXNamespace */ public ObjectName preRegister(MBeanServer server, ObjectName name) throws Exception { - if (objectName != null && ! objectName.equals(name)) - throw new IllegalStateException( + // need to synchronize to protect against multiple registration. + synchronized(this) { + if (objectName != null && ! objectName.equals(name)) + throw new IllegalStateException( "Already registered under another name: " + objectName); - objectName = validateHandlerName(name); - mbeanServer = server; + objectName = validateHandlerName(name); + mbeanServer = server; + } return name; } @@ -517,23 +520,23 @@ public class JMXNamespace * reuse JMXNamespace in order to implement sessions... * It is however only available for subclasses in this package. **/ - ObjectName validateHandlerName(ObjectName supliedName) { - if (supliedName == null) + ObjectName validateHandlerName(ObjectName suppliedName) { + if (suppliedName == null) throw new IllegalArgumentException("Must supply a valid name"); final String dirName = JMXNamespaces. - normalizeNamespaceName(supliedName.getDomain()); + normalizeNamespaceName(suppliedName.getDomain()); final ObjectName handlerName = JMXNamespaces.getNamespaceObjectName(dirName); - if (!supliedName.equals(handlerName)) + if (!suppliedName.equals(handlerName)) throw new IllegalArgumentException("invalid name space name: "+ - supliedName); - return supliedName; + suppliedName); + return suppliedName; } /** * This method is part of the {@link MBeanRegistration} interface. * The {@link JMXNamespace} class uses the {@link MBeanRegistration} - * interface in order to get a handle to the MBean server in which it is + * interface in order to get a reference to the MBean server in which it is * registered. *

* This method is called by the MBean server. Application classes should @@ -549,7 +552,7 @@ public class JMXNamespace /** * This method is part of the {@link MBeanRegistration} interface. * The {@link JMXNamespace} class uses the {@link MBeanRegistration} - * interface in order to get a handle to the MBean server in which it is + * interface in order to get a reference to the MBean server in which it is * registered. *

* This method is called by the MBean server. Application classes should @@ -573,8 +576,11 @@ public class JMXNamespace * @see MBeanRegistration#postDeregister MBeanRegistration */ public void postDeregister() { - mbeanServer = null; - objectName = null; + // need to synchronize to protect against multiple registration. + synchronized(this) { + mbeanServer = null; + objectName = null; + } } diff --git a/src/share/classes/javax/management/namespace/JMXNamespaces.java b/src/share/classes/javax/management/namespace/JMXNamespaces.java index b3eafd28a..429a9d466 100644 --- a/src/share/classes/javax/management/namespace/JMXNamespaces.java +++ b/src/share/classes/javax/management/namespace/JMXNamespaces.java @@ -266,11 +266,15 @@ public class JMXNamespaces { ObjectNameRouter.normalizeNamespacePath(namespace,false, true,false); try { + // We could use Util.newObjectName here - but throwing an + // IllegalArgumentException that contains just the supplied + // namespace instead of the whole ObjectName seems preferable. return ObjectName.getInstance(sourcePath+ NAMESPACE_SEPARATOR+":"+ JMXNamespace.TYPE_ASSIGNMENT); } catch (MalformedObjectNameException x) { - throw new IllegalArgumentException(namespace,x); + throw new IllegalArgumentException("Invalid namespace: " + + namespace,x); } } diff --git a/src/share/classes/javax/management/namespace/JMXRemoteNamespace.java b/src/share/classes/javax/management/namespace/JMXRemoteNamespace.java index 1639fd2fc..1e877e0ce 100644 --- a/src/share/classes/javax/management/namespace/JMXRemoteNamespace.java +++ b/src/share/classes/javax/management/namespace/JMXRemoteNamespace.java @@ -35,6 +35,7 @@ import java.io.IOException; import java.security.AccessControlException; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; @@ -125,10 +126,8 @@ public class JMXRemoteNamespace // the underlying JMXConnector. It is used in particular to maintain the // "connected" state in this MBean. // - private static class ConnectionListener implements NotificationListener { - private final JMXRemoteNamespace handler; - private ConnectionListener(JMXRemoteNamespace handler) { - this.handler = handler; + private class ConnectionListener implements NotificationListener { + private ConnectionListener() { } public void handleNotification(Notification notification, Object handback) { @@ -136,7 +135,11 @@ public class JMXRemoteNamespace return; final JMXConnectionNotification cn = (JMXConnectionNotification)notification; - handler.checkState(this,cn,(JMXConnector)handback); + final String type = cn.getType(); + if (JMXConnectionNotification.CLOSED.equals(type) + || JMXConnectionNotification.FAILED.equals(type)) { + checkState(this,cn,(JMXConnector)handback); + } } } @@ -188,7 +191,7 @@ public class JMXRemoteNamespace "Connected", "Emitted when the Connected state of this object changes"); - private static long seqNumber=0; + private static AtomicLong seqNumber = new AtomicLong(0); private final NotificationBroadcasterSupport broadcaster; private final ConnectionListener listener; @@ -237,7 +240,7 @@ public class JMXRemoteNamespace this.optionsMap = JMXNamespaceUtils.unmodifiableMap(optionsMap); // handles (dis)connection events - this.listener = new ConnectionListener(this); + this.listener = new ConnectionListener(); // XXX TODO: remove the probe, or simplify it. this.probed = false; @@ -313,8 +316,8 @@ public class JMXRemoteNamespace broadcaster.removeNotificationListener(listener, filter, handback); } - private static synchronized long getNextSeqNumber() { - return seqNumber++; + private static long getNextSeqNumber() { + return seqNumber.getAndIncrement(); } @@ -362,14 +365,18 @@ public class JMXRemoteNamespace // lock while evaluating the true value of the connected state, // while anyone might also call close() or connect() from a // different thread. - // // The method switchConnection() (called from here too) also has the - // same kind of complex logic. + // same kind of complex logic: // // We use the JMXConnector has a handback to the notification listener // (emittingConnector) in order to be able to determine whether the // notification concerns the current connector in use, or an older - // one. + // one. The 'emittingConnector' is the connector from which the + // notification originated. This could be an 'old' connector - as + // closed() and connect() could already have been called before the + // notification arrived. So what we do is to compare the + // 'emittingConnector' with the current connector, to see if the + // notification actually comes from the curent connector. // boolean remove = false; @@ -486,14 +493,12 @@ public class JMXRemoteNamespace } } - private void closeall(JMXConnector... a) { - for (JMXConnector c : a) { - try { - if (c != null) c.close(); - } catch (Exception x) { - // OK: we're gonna throw the original exception later. - LOG.finest("Ignoring exception when closing connector: "+x); - } + private void close(JMXConnector c) { + try { + if (c != null) c.close(); + } catch (Exception x) { + // OK: we're gonna throw the original exception later. + LOG.finest("Ignoring exception when closing connector: "+x); } } @@ -640,10 +645,10 @@ public class JMXRemoteNamespace msc = aconn.getMBeanServerConnection(); aconn.addConnectionNotificationListener(listener,null,aconn); } catch (IOException io) { - closeall(aconn); + close(aconn); throw io; } catch (RuntimeException x) { - closeall(aconn); + close(aconn); throw x; } diff --git a/src/share/classes/javax/management/namespace/MBeanServerConnectionWrapper.java b/src/share/classes/javax/management/namespace/MBeanServerConnectionWrapper.java index f74785ffd..7eeda961e 100644 --- a/src/share/classes/javax/management/namespace/MBeanServerConnectionWrapper.java +++ b/src/share/classes/javax/management/namespace/MBeanServerConnectionWrapper.java @@ -28,7 +28,6 @@ package javax.management.namespace; import com.sun.jmx.mbeanserver.Util; import java.io.IOException; import java.io.ObjectInputStream; -import java.security.AccessController; import java.util.Set; import javax.management.Attribute; diff --git a/test/javax/management/namespace/Wombat.java b/test/javax/management/namespace/Wombat.java index 03dbd020e..bef648c0f 100644 --- a/test/javax/management/namespace/Wombat.java +++ b/test/javax/management/namespace/Wombat.java @@ -68,7 +68,12 @@ public class Wombat extends StandardMBean } public Wombat() throws NotCompliantMBeanException { - super(WombatMBean.class); + this(WombatMBean.class); + } + + public Wombat(Class clazz) + throws NotCompliantMBeanException { + super(clazz); final Random r = new Random(); seed = ((r.nextLong() % MAX_SEED) + MAX_SEED)%MAX_SEED; period = 200 + (((r.nextLong()%80)+80)%80)*10; -- GitLab