6745832: jmx namespaces: Some refactoring/commenting would improve code readability.

Reviewed-by: emcmanus

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<Runnable> 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<Runnable> postQueue) {
-        dispatcher.removeNamespace(logicalName, namespace, postQueue);
+        dispatcher.removeInterceptorFor(logicalName, namespace, postQueue);
     }
 
     /**

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<Runnable> 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<Runnable> 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<ObjectInstance> queryMBeans(ObjectName name, QueryExp query) {
-        final QueryInterceptor mbs =
+    public final Set<ObjectInstance> 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<ObjectName> queryNames(ObjectName name, QueryExp query) {
-        final QueryInterceptor mbs =
+    public final Set<ObjectName> 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);
     }
 
 }

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<T> 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<domains.length;i++) {
             if (domains[i].equals(key))
                 throw new IllegalArgumentException("domain "+key+
@@ -192,37 +191,72 @@ class DomainDispatchInterceptor
 
     @Override
     final MBeanServer getInterceptorOrNullFor(ObjectName name) {
-        if (name == null) return localNamespace;
+
+        if (name == null) return nextInterceptor;
+
         final String domain = name.getDomain();
-        if (domain.endsWith(NAMESPACE_SEPARATOR)) return localNamespace;
-        if (domain.contains(NAMESPACE_SEPARATOR)) return null;
-        final String localDomain = domain;
-        if (isLocalHandlerNameFor(localDomain,name)) {
+        if (domain.endsWith(NAMESPACE_SEPARATOR))
+            return nextInterceptor; // This can be a namespace handler.
+        if (domain.contains(NAMESPACE_SEPARATOR))
+            return null; // shouldn't reach here.
+        if (isLocalHandlerNameFor(domain,name)) {
+            // This is the name of a JMXDomain MBean. Return nextInterceptor.
             LOG.finer("dispatching to local namespace");
-            return localNamespace;
+            return nextInterceptor;
         }
-        final DomainInterceptor ns = getInterceptor(localDomain);
+
+        final DomainInterceptor ns = getInterceptor(domain);
         if (ns == null) {
+            // no JMXDomain found for that domain - return nextInterceptor.
             if (LOG.isLoggable(Level.FINER)) {
-                LOG.finer("dispatching to local namespace: " + localDomain);
+                LOG.finer("dispatching to local namespace: " + domain);
             }
             return getNextInterceptor();
         }
+
         if (LOG.isLoggable(Level.FINER)) {
-            LOG.finer("dispatching to domain: " + localDomain);
+            LOG.finer("dispatching to domain: " + domain);
         }
         return ns;
     }
 
+    // This method returns true if the given pattern must be evaluated against
+    // several interceptors. This happens when either:
+    //
+    //   a) the pattern can select several domains (it's null, or it's a
+    //        domain pattern)
+    //   or b) it's not a domain pattern, but it might select the name of a
+    //        JMXDomain MBean in charge of that domain. Since the JMXDomain
+    //        MBean is located in the nextInterceptor, the pattern might need
+    //        to be evaluated on two interceptors.
+    //
+    // 1. When this method returns false, the query is evaluated on a single
+    // interceptor:
+    //    The interceptor for pattern.getDomain(), if there is one,
+    //    or the next interceptor, if there is none.
+    //
+    // 2. When this method returns true, we loop over all the domain
+    // interceptors:
+    //    in the list, and if the domain pattern matches the interceptor domain
+    //    we evaluate the query on that interceptor and aggregate the results.
+    //    Eventually we also evaluate the pattern against the next interceptor.
+    //
+    // See getInterceptorForQuery below.
+    //
     private boolean multipleQuery(ObjectName pattern) {
+        // case a) above
         if (pattern == null) return true;
         if (pattern.isDomainPattern()) return true;
 
         try {
+            // case b) above.
+            //
             // This is a bit of a hack. If there's any chance that a JMXDomain
             // MBean name is selected by the given pattern then we must include
             // the local namespace in our search.
-            // Returning true will have this effect.
+            //
+            // Returning true will have this effect. see 2. above.
+            //
             if (pattern.apply(ALL_DOMAINS.withDomain(pattern.getDomain())))
                 return true;
         } catch (MalformedObjectNameException x) {
@@ -253,7 +287,7 @@ class DomainDispatchInterceptor
         // We don't have a virtual domain. Send to local domains.
         if (LOG.isLoggable(Level.FINER))
              LOG.finer("dispatching to local namespace: " + domain);
-        return new QueryInterceptor(localNamespace);
+        return new QueryInterceptor(nextInterceptor);
     }
 
     @Override
@@ -288,7 +322,7 @@ class DomainDispatchInterceptor
 
     @Override
     final DefaultMBeanServerInterceptor getNextInterceptor() {
-        return localNamespace;
+        return nextInterceptor;
     }
 
     /**
@@ -298,11 +332,11 @@ class DomainDispatchInterceptor
     @Override
     public String[] getDomains() {
         // A JMXDomain is registered in its own domain.
-        // Therefore, localNamespace.getDomains() contains all domains.
-        // In addition, localNamespace will perform the necessary
+        // Therefore, nextInterceptor.getDomains() contains all domains.
+        // In addition, nextInterceptor will perform the necessary
         // MBeanPermission checks for getDomains().
         //
-        return localNamespace.getDomains();
+        return nextInterceptor.getDomains();
     }
 
     /**
@@ -310,13 +344,13 @@ class DomainDispatchInterceptor
      */
     @Override
     public Integer getMBeanCount() {
-        int count = getNextInterceptor().getMBeanCount().intValue();
+        int count = getNextInterceptor().getMBeanCount();
         final String[] keys = getKeys();
         for (String key:keys) {
             final MBeanServer mbs = getInterceptor(key);
             if (mbs == null) continue;
-            count += mbs.getMBeanCount().intValue();
+            count += mbs.getMBeanCount();
         }
-        return Integer.valueOf(count);
+        return count;
     }
 }

src/share/classes/com/sun/jmx/interceptor/NamespaceDispatchInterceptor.java

@@ -61,7 +61,7 @@ public class NamespaceDispatchInterceptor
     private static final int NAMESPACE_SEPARATOR_LENGTH =
             NAMESPACE_SEPARATOR.length();
 
-    private final DomainDispatchInterceptor localNamespace;
+    private final DomainDispatchInterceptor nextInterceptor;
     private final String           serverName;
 
     /**
@@ -84,7 +84,7 @@ public class NamespaceDispatchInterceptor
                                MBeanServerDelegate delegate,
                                MBeanInstantiator   instantiator,
                                Repository          repository)  {
-           localNamespace = new DomainDispatchInterceptor(outer,delegate,
+           nextInterceptor = new DomainDispatchInterceptor(outer,delegate,
                    instantiator,repository,this);
            serverName = Util.getMBeanServerSecurityName(delegate);
     }
@@ -94,21 +94,21 @@ public class NamespaceDispatchInterceptor
      * Get first name space in ObjectName path. Ignore leading namespace
      * separators.
      **/
-    public static String getFirstNamespace(ObjectName name) {
+    static String getFirstNamespace(ObjectName name) {
         if (name == null) return "";
         final String domain = name.getDomain();
         if (domain.equals("")) return "";
 
+        // skip leading separators
         int first = 0;
-        int end = domain.indexOf(NAMESPACE_SEPARATOR,first);
-        while (end == first) {
-            first = end+NAMESPACE_SEPARATOR_LENGTH;
-            end = domain.indexOf(NAMESPACE_SEPARATOR,first);
-            if (end == -1) break;
-        }
+        while (domain.startsWith(NAMESPACE_SEPARATOR,first))
+            first += NAMESPACE_SEPARATOR_LENGTH;
 
-        if (end == -1) return "";
+        // go to next separator
+        final int end = domain.indexOf(NAMESPACE_SEPARATOR,first);
+        if (end == -1) return ""; // no namespace
 
+        // This is the first element in the namespace path.
         final String namespace = domain.substring(first,end);
 
         return namespace;
@@ -143,7 +143,7 @@ public class NamespaceDispatchInterceptor
         if (namespace.equals("") || isLocalHandlerNameFor(namespace,name) ||
             name.getDomain().equals(namespace+NAMESPACE_SEPARATOR)) {
             LOG.finer("dispatching to local name space");
-            return localNamespace;
+            return nextInterceptor;
         }
         final NamespaceInterceptor ns = getInterceptor(namespace);
         if (LOG.isLoggable(Level.FINER)) {
@@ -162,7 +162,7 @@ public class NamespaceDispatchInterceptor
         if (namespace.equals("") || isLocalHandlerNameFor(namespace,pattern) ||
             pattern.getDomain().equals(namespace+NAMESPACE_SEPARATOR)) {
             LOG.finer("dispatching to local name space");
-            return new QueryInterceptor(localNamespace);
+            return new QueryInterceptor(nextInterceptor);
         }
         final NamespaceInterceptor ns = getInterceptor(namespace);
         if (LOG.isLoggable(Level.FINER)) {
@@ -202,7 +202,7 @@ public class NamespaceDispatchInterceptor
 
     @Override
     final DomainDispatchInterceptor getNextInterceptor() {
-        return localNamespace;
+        return nextInterceptor;
     }
 
     /**
@@ -211,25 +211,25 @@ public class NamespaceDispatchInterceptor
      */
     @Override
     public String[] getDomains() {
-        return localNamespace.getDomains();
+        return nextInterceptor.getDomains();
     }
 
     @Override
-    public void addNamespace(ObjectName name, JMXNamespace handler,
+    public void addInterceptorFor(ObjectName name, JMXNamespace handler,
             Queue<Runnable> 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<Runnable> 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);
     }
 
 

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 <K, V> Map<K, V> 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());
        }
     }

src/share/classes/com/sun/jmx/namespace/DomainInterceptor.java

@@ -85,7 +85,7 @@ public class DomainInterceptor extends HandlerInterceptor<JMXDomain> {
 
         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<JMXDomain> {
                 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<JMXDomain> {
         super(handler);
         this.domainName = domainName;
         this.serverName = serverName;
+        ALL = Util.newObjectName(domainName+":*");
     }
 
     @Override
@@ -118,27 +119,27 @@ public class DomainInterceptor extends HandlerInterceptor<JMXDomain> {
                 ", 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<JMXDomain> {
             if (l == null) return;
             mbsListener = null;
         }
-        getNamespace().removeMBeanServerNotificationListener(l);
+        getHandlerInterceptorMBean().removeMBeanServerNotificationListener(l);
     }
 
-    public void addPostRegisterTask(Queue<Runnable> queue,
+    public final void addPostRegisterTask(Queue<Runnable> 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<JMXDomain> {
                 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<Runnable> queue,
+    public final void addPostDeregisterTask(Queue<Runnable> 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<JMXDomain> {
                 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<JMXDomain> {
         return targetName;
     }
 
+    // No name conversion for JMXDomains...
     @Override
     protected ObjectName toTarget(ObjectName sourceName) {
         return sourceName;
@@ -255,16 +259,16 @@ public class DomainInterceptor extends HandlerInterceptor<JMXDomain> {
             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<ObjectName> 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<JMXDomain> {
             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<ObjectInstance> empty = Collections.emptySet();
-        return empty;
     }
 
     @Override
@@ -306,7 +307,7 @@ public class DomainInterceptor extends HandlerInterceptor<JMXDomain> {
     // 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<JMXDomain> {
     @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<JMXDomain> {
         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<JMXDomain> {
     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+

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}.
  * <p><b>
  * This API is a Sun internal API and is subject to changes without notice.
  * </b></p>
@@ -90,6 +90,12 @@ public abstract class HandlerInterceptor<T extends JMXNamespace>
         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<T extends JMXNamespace>
          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<T extends JMXNamespace>
                     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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // From MBeanServer: catch & handles IOException
+    // From MBeanServerConnection: catch & handles IOException
     @Override
     public String getDefaultDomain() {
         try {
@@ -193,7 +201,7 @@ public abstract class HandlerInterceptor<T extends JMXNamespace>
         }
     }
 
-    // From MBeanServer: catch & handles IOException
+    // From MBeanServerConnection: catch & handles IOException
     @Override
     public String[] getDomains() {
         try {
@@ -203,7 +211,7 @@ public abstract class HandlerInterceptor<T extends JMXNamespace>
         }
     }
 
-    // From MBeanServer: catch & handles IOException
+    // From MBeanServerConnection: catch & handles IOException
     @Override
     public Integer getMBeanCount() {
         try {
@@ -213,7 +221,7 @@ public abstract class HandlerInterceptor<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // From MBeanServer: catch & handles IOException
+    // From MBeanServerConnection: catch & handles IOException
     @Override
     public Set<ObjectName> queryNames(ObjectName name, QueryExp query) {
         try {
@@ -236,7 +244,7 @@ public abstract class HandlerInterceptor<T extends JMXNamespace>
         }
     }
 
-    // From MBeanServer: catch & handles IOException
+    // From MBeanServerConnection: catch & handles IOException
     @Override
     public Set<ObjectInstance> queryMBeans(ObjectName name, QueryExp query) {
         try {
@@ -246,7 +254,7 @@ public abstract class HandlerInterceptor<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // 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<T extends JMXNamespace>
         }
     }
 
-    // From MBeanServer: catch & handles IOException
+    // From MBeanServerConnection: catch & handles IOException
     @Override
     public Object invoke(ObjectName name, String operationName, Object[] params,
                 String[] signature)

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<K,V>();
     }
 
-    /** 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 <K,V> Map<K,V> filterMap(Map<K,V> map, K... keys) {
-        final Map<K,V> filtered;
-        filtered=new HashMap<K,V>(map);
-        for (K key : keys) {
-            filtered.remove(key);
-        }
-        return unmodifiableMap(filtered);
-    }
-
     // returns un unmodifiable view of a map.
     public static <K,V> Map<K,V> unmodifiableMap(Map<K,V> aMap) {
         if (aMap == null || aMap.isEmpty())

src/share/classes/com/sun/jmx/namespace/NamespaceInterceptor.java

@@ -54,10 +54,6 @@ import javax.management.namespace.JMXNamespacePermission;
  */
 public class NamespaceInterceptor extends HandlerInterceptor<JMXNamespace> {
 
-    /**
-     * 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");
 

src/share/classes/com/sun/jmx/namespace/RoutingConnectionProxy.java

@@ -45,6 +45,9 @@ import javax.management.namespace.JMXNamespaces;
  * </b></p>
  * @since 1.7
  */
+// See class hierarchy and detailled explanations in RoutingProxy in this
+// package.
+//
 public class RoutingConnectionProxy
         extends RoutingProxy<MBeanServerConnection> {
 
@@ -93,40 +96,28 @@ public class RoutingConnectionProxy
                targetNs+"\", "+forwardsContext+")";
     }
 
+    static final RoutingProxyFactory
+            <MBeanServerConnection,RoutingConnectionProxy>
+        FACTORY = new RoutingProxyFactory
+        <MBeanServerConnection,RoutingConnectionProxy>() {
+
+        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);
     }
 
 }

src/share/classes/com/sun/jmx/namespace/RoutingMBeanServerConnection.java

@@ -83,18 +83,32 @@ public abstract class RoutingMBeanServerConnection<T extends MBeanServerConnecti
     }
 
     /**
-     * Returns the wrapped source connection.
+     * Returns the wrapped source connection. The {@code source} connection
+     * is a connection to the MBeanServer that contains the actual MBean.
+     * In the case of cascading, that would be a connection to the sub
+     * agent.
      **/
     protected abstract T source() throws IOException;
 
     /**
      * Converts a target ObjectName to a source ObjectName.
+     * The target ObjectName is the name of the MBean in the mount point
+     * target. In the case of cascading, that would be the name of the
+     * MBean in the master agent. So if a subagent S containing an MBean
+     * named "X" is mounted in the target namespace "foo//" of a master agent M,
+     * the source is S, the target is "foo//" in M, the source name is "X", and
+     * the target name is "foo//X".
+     * In the case of cascading - such as in NamespaceInterceptor, this method
+     * will convert "foo//X" (the targetName) into "X", the source name.
      **/
     protected abstract ObjectName toSource(ObjectName targetName)
         throws MalformedObjectNameException;
 
     /**
      * Converts a source ObjectName to a target ObjectName.
+     * (see description of toSource above for explanations)
+     * In the case of cascading - such as in NamespaceInterceptor, this method
+     * will convert "X" (the sourceName) into "foo//X", the target name.
      **/
     protected abstract ObjectName toTarget(ObjectName sourceName)
         throws MalformedObjectNameException;
@@ -142,44 +156,6 @@ public abstract class RoutingMBeanServerConnection<T extends MBeanServerConnecti
         return new RuntimeOperationsException(x2);
     }
 
-    /**
-     * 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 <namespace>//<ObjectName>}.
-     * @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 <namespace>//<ObjectName>}.
-     * @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<T extends MBeanServerConnecti
         }
     }
 
-   /**
-     * 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 <namespace>//<ObjectName>}.
-     * @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<T extends MBeanServerConnecti
         return result;
     }
 
-    /**
-     * 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 <namespace>//<ObjectName>}.
-     * @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<T extends MBeanServerConnecti
         }
     }
 
+    // from MBeanServerConnection
+    public String getDefaultDomain() throws IOException {
+        try {
+            return source().getDefaultDomain();
+        } catch (RuntimeException ex) {
+            throw makeCompliantRuntimeException(ex);
+        }
+    }
+
+    //----------------------------------------------------------------------
+    // Hooks for checking permissions
+    //----------------------------------------------------------------------
+
+    /**
+     * 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 <namespace>//<ObjectName>}.
+     * @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 <namespace>//<ObjectName>}.
+     * @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 <namespace>//<ObjectName>}.
+     * @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 <namespace>//<ObjectName>}.
+     * @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<T extends MBeanServerConnecti
     String[] checkDomains(String[] domains, String action) {
         return domains;
     }
-
-    // from MBeanServerConnection
-    public String getDefaultDomain() throws IOException {
-        try {
-            return source().getDefaultDomain();
-        } catch (RuntimeException ex) {
-            throw makeCompliantRuntimeException(ex);
-        }
-    }
-
 }

src/share/classes/com/sun/jmx/namespace/RoutingProxy.java

@@ -30,31 +30,97 @@ import java.io.IOException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import javax.management.AttributeNotFoundException;
-import javax.management.InstanceNotFoundException;
 import javax.management.MBeanException;
 import javax.management.MBeanRegistrationException;
 
 import javax.management.MBeanServerConnection;
 import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
-import javax.management.ReflectionException;
 import javax.management.namespace.JMXNamespaces;
 
 
 /**
- * An RoutingProxy narrows on a given name space in a
+ * A RoutingProxy narrows on a given name space in a
  * source object implementing MBeanServerConnection.
  * It is used to implement
  * {@code JMXNamespaces.narrowToNamespace(...)}.
  * This abstract class has two concrete subclasses:
- * <p>{@link RoutingConnectionProxy}: to cd in an MBeanServerConnection.</p>
- * <p>{@link RoutingServerProxy}: to cd in an MBeanServer.</p>
+ * <p>{@link RoutingConnectionProxy}: to narrow down into an
+ *    MBeanServerConnection.</p>
+ * <p>{@link RoutingServerProxy}: to narrow down into an MBeanServer.</p>
  * <p><b>
  * This API is a Sun internal API and is subject to changes without notice.
  * </b></p>
  * @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-we-narrow-down-to> // namespace 'to add'
+//
+// For a RoutingProxy used in a ClientContext operation, we have:
+//     targetNs=<encoded-context> // 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<T extends MBeanServerConnection>
         extends RoutingMBeanServerConnection<T> {
 
@@ -179,17 +245,11 @@ public abstract class RoutingProxy<T extends MBeanServerConnection>
              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<T extends MBeanServerConnection>
                     (" mounted on targetNs="+targetNs));
     }
 
+    // Creates an instance of a subclass 'R' of RoutingProxy<T>
+    // RoutingServerProxy and RoutingConnectionProxy have their own factory
+    // instance.
+    static interface RoutingProxyFactory<T extends MBeanServerConnection,
+            R extends RoutingProxy<T>> {
+            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 <T extends MBeanServerConnection, R extends RoutingProxy<T>>
+           R cd(Class<R> routingProxyClass,
+              RoutingProxyFactory<T,R> 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);
+    }
 }

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<MBeanServer>
         implements MBeanServer {
@@ -564,39 +567,24 @@ public class RoutingServerProxy
         }
     }
 
+    static final RoutingProxyFactory<MBeanServer,RoutingServerProxy>
+        FACTORY = new RoutingProxyFactory<MBeanServer,RoutingServerProxy>() {
+
+        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);
     }
 }

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;
     }
 
     /**

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.
      * <p>
      * 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.
      * <p>
      * 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.
      * <p>
      * 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;
+        }
     }
 
 

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);
         }
     }
 

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;
         }
 

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;

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<? extends WombatMBean> 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;