Issue #3729 concurrent JNDI access

+ concurrent map for the NamingContext bindings
+ refactored duplicate code into common methods
+ simplified deepBindSupport

Signed-off-by: Greg Wilkins <gregw@webtide.com>

jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java

@@ -18,12 +18,10 @@
 
 package org.eclipse.jetty.jndi;
 
-
 import java.io.IOException;
 import java.util.Hashtable;
 import java.util.Map;
 import java.util.WeakHashMap;
-
 import javax.naming.Context;
 import javax.naming.Name;
 import javax.naming.NameParser;
@@ -77,8 +75,6 @@ public class ContextFactory implements ObjectFactory
      * when finding the comp context.
      */
     private static final ThreadLocal<ClassLoader> __threadClassLoader = new ThreadLocal<ClassLoader>();
-    
-
 
     /**
      * Find or create a context which pertains to a classloader.

jetty-jndi/src/main/java/org/eclipse/jetty/jndi/InitialContextFactory.java

@@ -18,10 +18,8 @@
 
 package org.eclipse.jetty.jndi;
 
-
 import java.util.Hashtable;
 import java.util.Properties;
-
 import javax.naming.CompoundName;
 import javax.naming.Context;
 import javax.naming.Name;

jetty-jndi/src/main/java/org/eclipse/jetty/jndi/NamingContext.java

@@ -22,11 +22,10 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.List;
-import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
-
+import java.util.concurrent.ConcurrentMap;
 import javax.naming.Binding;
 import javax.naming.CompoundName;
 import javax.naming.Context;
@@ -56,7 +55,7 @@ import org.eclipse.jetty.util.log.Logger;
  * All Names are expected to be Compound, not Composite.
  */
 @SuppressWarnings("unchecked")
-public class NamingContext implements Context, Cloneable, Dumpable
+public class NamingContext implements Context, Dumpable
 {
     private final static Logger __log=NamingUtil.__log;
     private final static List<Binding> __empty = Collections.emptyList();
@@ -64,9 +63,22 @@ public class NamingContext implements Context, Cloneable, Dumpable
     public static final String LOCK_PROPERTY = "org.eclipse.jetty.jndi.lock";
     public static final String UNLOCK_PROPERTY = "org.eclipse.jetty.jndi.unlock";
 
-    protected final Hashtable<String,Object> _env = new Hashtable<String,Object>();
+    /*
-    private boolean _supportDeepBinding = false;
+     * The env is stored as a Hashtable to be compatible with the API.
-    protected Map<String,Binding> _bindings = new HashMap<String,Binding>();
+     * There is no need for concurrent protection (see Concurrent Access section
+     * of the {@link Context} javadoc), so multiple threads acting on the same
+     * Context env need to self - mutually exclude.
+     */
+    protected final Hashtable<String,Object> _env = new Hashtable<>();
+
+    /*
+     * This contexts bindings are stored as a ConcurrentHashMap.
+     * There is generally no need for concurrent protection (see Concurrent Access section
+     * of the {@link Context} javadoc), However, the NamingContext is used for root contexts and
+     * we do not mutually exclude when initializing, so instead we do make the bindings
+     * thread safe (unlike the env where we expect users to respect the Concurrent Access requirements).
+     */
+    protected final ConcurrentMap<String,Binding> _bindings;
 
     protected NamingContext _parent = null;
     protected String _name = null;
@@ -99,7 +111,7 @@ public class NamingContext implements Context, Cloneable, Dumpable
     /**
      * Constructor
      *
-     * @param env environment properties
+     * @param env environment properties which are copied into this Context's environment
      * @param name relative name of this context
      * @param parent immediate ancestor Context (can be null)
      * @param parser NameParser for this Context
@@ -108,53 +120,48 @@ public class NamingContext implements Context, Cloneable, Dumpable
                          String name,
                          NamingContext parent,
                          NameParser parser)
+    {
+        this(env, name, parent, parser, null);
+    }
+
+    private NamingContext(Hashtable<String,Object> env,
+                         String name,
+                         NamingContext parent,
+                         NameParser parser,
+                         ConcurrentMap<String, Binding> bindings)
     {
         if (env != null)
-        {
             _env.putAll(env);
-            // look for deep binding support in _env
-            Object support = _env.get(DEEP_BINDING);
-            if (support == null)
-                _supportDeepBinding = false;
-            else
-                _supportDeepBinding = support != null?Boolean.parseBoolean(support.toString()):false;
-        }
-        else
-        {
-            // no env?  likely this is a root context (java or local) that
-            // was created without an _env.  Look for a system property.
-            String value = System.getProperty(DEEP_BINDING,"false");
-            _supportDeepBinding = Boolean.parseBoolean(value);
-            // put what we discovered into the _env for later sub-contexts
-            // to utilize.
-            _env.put(DEEP_BINDING,_supportDeepBinding);
-        }
         _name = name;
         _parent = parent;
         _parser = parser;
+        _bindings = bindings==null ? new ConcurrentHashMap<>() : bindings;
         if(__log.isDebugEnabled())
-            __log.debug("supportDeepBinding={}",_supportDeepBinding);
+            __log.debug("new {}", this);
     }
 
-
-    /*------------------------------------------------*/
     /**
-     * Clone this NamingContext
+     * @return A shallow copy of the Context with the same bindings, but a copy of the Env
-     *
-     * @return copy of this NamingContext
-     * @exception CloneNotSupportedException if an error occurs
      */
-    @Override
+    public NamingContext shallowCopy()
-    public Object clone ()
-        throws CloneNotSupportedException
     {
-        NamingContext ctx = (NamingContext)super.clone();
+        return new NamingContext(_env, _name, _parent, _parser, _bindings);
-        ctx._env.putAll(_env);
-        ctx._bindings.putAll(_bindings);
-        return ctx;
     }
 
 
+    public boolean isDeepBindingSupported()
+    {
+        // look for deep binding support in _env
+        Object support = _env.get(DEEP_BINDING);
+        if (support!=null)
+            return Boolean.parseBoolean(String.valueOf(support));
+
+        if (_parent!=null)
+            return _parent.isDeepBindingSupported();
+        // probably a root context
+        return Boolean.parseBoolean(System.getProperty(DEEP_BINDING,"false"));
+    }
+
     /*------------------------------------------------*/
     /**
      * Getter for _name
@@ -190,20 +197,54 @@ public class NamingContext implements Context, Cloneable, Dumpable
         if(env == null)
             return;
         _env.putAll(env);
-        _supportDeepBinding = _env.containsKey(DEEP_BINDING);
     }
 
-
+    private Object dereference(Object ctx, String firstComponent) throws NamingException
-    public Map<String,Binding> getBindings ()
     {
-        return _bindings;
+        if (ctx instanceof Reference)
+        {
+            //deference the object
+            try
+            {
+                return NamingManager.getObjectInstance(ctx, _parser.parse(firstComponent), this, _env);
+            }
+            catch (NamingException e)
+            {
+                throw e;
+            }
+            catch (Exception e)
+            {
+                __log.warn("",e);
+                throw new NamingException (e.getMessage());
+            }
+        }
+        return ctx;
     }
 
-    public void setBindings(Map<String,Binding> bindings)
+    private Context getContext(Name cname) throws NamingException
     {
-        _bindings = bindings;
+        String firstComponent = cname.get(0);
+
+        if (firstComponent.equals(""))
+            return this;
+
+        Binding binding = getBinding (firstComponent);
+        if (binding == null)
+        {
+            NameNotFoundException nnfe = new NameNotFoundException(firstComponent + " is not bound");
+            nnfe.setRemainingName(cname);
+            throw nnfe;
+        }
+
+        Object ctx = dereference(binding.getObject(), firstComponent);
+
+        if (!(ctx instanceof Context))
+            throw new NotContextException(firstComponent + " not a context in " + this.getNameInNamespace());
+
+        return (Context)ctx;
     }
 
+
     /*------------------------------------------------*/
     /**
      * Bind a name to an object
@@ -244,7 +285,8 @@ public class NamingContext implements Context, Cloneable, Dumpable
         }
         else
         {
-            if(__log.isDebugEnabled())__log.debug("Checking for existing binding for name="+cname+" for first element of name="+cname.get(0));
+            if(__log.isDebugEnabled())
+                __log.debug("Checking for existing binding for name="+cname+" for first element of name="+cname.get(0));
 
             //walk down the subcontext hierarchy
             //need to ignore trailing empty "" name components
@@ -256,13 +298,13 @@ public class NamingContext implements Context, Cloneable, Dumpable
                 ctx = this;
             else
             {
-
                 Binding  binding = getBinding (firstComponent);
-                if (binding == null) {
+                if (binding == null)
-                    if (_supportDeepBinding)
+                {
+                    if (isDeepBindingSupported())
                     {
                         Name subname = _parser.parse(firstComponent);
-                        Context subctx = new NamingContext((Hashtable)_env.clone(),firstComponent,this,_parser);
+                        Context subctx = new NamingContext(_env,firstComponent,this,_parser, null);
                         addBinding(subname,subctx);
                         binding = getBinding(subname);
                     }
@@ -272,28 +314,9 @@ public class NamingContext implements Context, Cloneable, Dumpable
                     }
                 }
 
-                ctx = binding.getObject();
+                ctx = dereference(binding.getObject(),firstComponent);
-
-                if (ctx instanceof Reference)
-                {
-                    //deference the object
-                    try
-                    {
-                        ctx = NamingManager.getObjectInstance(ctx, getNameParser("").parse(firstComponent), this, _env);
-                    }
-                    catch (NamingException e)
-                    {
-                        throw e;
-                    }
-                    catch (Exception e)
-                    {
-                        __log.warn("",e);
-                        throw new NamingException (e.getMessage());
-                    }
-                }
             }
 
-
             if (ctx instanceof Context)
             {
                 ((Context)ctx).bind (cname.getSuffix(1), obj);
@@ -354,57 +377,14 @@ public class NamingContext implements Context, Cloneable, Dumpable
             if (binding != null)
                 throw new NameAlreadyBoundException (cname.toString());
 
-            Context ctx = new NamingContext ((Hashtable)_env.clone(), cname.get(0), this, _parser);
+            Context ctx = new NamingContext (_env, cname.get(0), this, _parser);
             addBinding (cname, ctx);
             return ctx;
         }
 
-
+        return getContext(cname).createSubcontext (cname.getSuffix(1));
-        //If the name has multiple subcontexts, walk the hierarchy by
-        //fetching the first one. All intermediate subcontexts in the
-        //name must already exist.
-        String firstComponent = cname.get(0);
-        Object ctx = null;
-
-        if (firstComponent.equals(""))
-            ctx = this;
-        else
-        {
-            Binding binding = getBinding (firstComponent);
-            if (binding == null)
-                throw new NameNotFoundException (firstComponent + " is not bound");
-
-            ctx = binding.getObject();
-
-            if (ctx instanceof Reference)
-            {
-                //deference the object
-                if(__log.isDebugEnabled())__log.debug("Object bound at "+firstComponent +" is a Reference");
-                try
-                {
-                    ctx = NamingManager.getObjectInstance(ctx, getNameParser("").parse(firstComponent), this, _env);
-                }
-                catch (NamingException e)
-                {
-                    throw e;
-                }
-                catch (Exception e)
-                {
-                    __log.warn("",e);
-                    throw new NamingException (e.getMessage());
-                }
-            }
-        }
-
-        if (ctx instanceof Context)
-        {
-            return ((Context)ctx).createSubcontext (cname.getSuffix(1));
-        }
-        else
-            throw new NotContextException (firstComponent +" is not a Context");
     }
 
-
     /*------------------------------------------------*/
     /**
      * Create a Context as a child of this one
@@ -420,8 +400,6 @@ public class NamingContext implements Context, Cloneable, Dumpable
         return createSubcontext(_parser.parse(name));
     }
 
-
-
     /*------------------------------------------------*/
     /**
      *
@@ -436,8 +414,6 @@ public class NamingContext implements Context, Cloneable, Dumpable
         removeBinding(_parser.parse(name));
     }
 
-
-
     /*------------------------------------------------*/
     /**
      *
@@ -468,14 +444,10 @@ public class NamingContext implements Context, Cloneable, Dumpable
 
         if ((cname == null) || (cname.size() == 0))
         {
-            __log.debug("Null or empty name, returning copy of this context");
+            __log.debug("Null or empty name, returning shallowCopy of this context");
-            NamingContext ctx = new NamingContext (_env, _name, _parent, _parser);
+            return shallowCopy();
-            ctx._bindings=_bindings;
-            return ctx;
         }
 
-
-
         if (cname.size() == 1)
         {
             Binding binding = getBinding (cname);
@@ -486,7 +458,6 @@ public class NamingContext implements Context, Cloneable, Dumpable
                 throw nnfe;
             }
 
-
             Object o = binding.getObject();
 
             //handle links by looking up the link
@@ -505,7 +476,7 @@ public class NamingContext implements Context, Cloneable, Dumpable
             }
             else if (o instanceof Reference)
             {
-                //deference the object
+                // TODO use deference ??
                 try
                 {
                     return NamingManager.getObjectInstance(o, cname, this, _env);
@@ -524,54 +495,9 @@ public class NamingContext implements Context, Cloneable, Dumpable
                 return o;
         }
 
-        //it is a multipart name, recurse to the first subcontext
+        return getContext(cname).lookup (cname.getSuffix(1));
-
-        String firstComponent = cname.get(0);
-        Object ctx = null;
-
-        if (firstComponent.equals(""))
-            ctx = this;
-        else
-        {
-
-            Binding binding = getBinding (firstComponent);
-            if (binding == null)
-            {
-                NameNotFoundException nnfe = new NameNotFoundException();
-                nnfe.setRemainingName(cname);
-                throw nnfe;
-            }
-
-            //as we have bound a reference to an object factory
-            //for the component specific contexts
-            //at "comp" we need to resolve the reference
-            ctx = binding.getObject();
-
-            if (ctx instanceof Reference)
-            {
-                //deference the object
-                try
-                {
-                    ctx = NamingManager.getObjectInstance(ctx, getNameParser("").parse(firstComponent), this, _env);
-                }
-                catch (NamingException e)
-                {
-                    throw e;
-                }
-                catch (Exception e)
-                {
-                    __log.warn("",e);
-                    throw new NamingException (e.getMessage());
-                }
-            }
-        }
-        if (!(ctx instanceof Context))
-            throw new NotContextException();
-
-        return ((Context)ctx).lookup (cname.getSuffix(1));
     }
 
-
     /*------------------------------------------------*/
     /**
      * Lookup binding of an object by name
@@ -587,8 +513,6 @@ public class NamingContext implements Context, Cloneable, Dumpable
         return lookup (_parser.parse(name));
     }
 
-
-
     /*------------------------------------------------*/
     /**
      * Lookup link bound to name
@@ -603,12 +527,11 @@ public class NamingContext implements Context, Cloneable, Dumpable
     {
         Name cname = toCanonicalName(name);
 
-        if (cname == null)
+        if (cname == null || name.isEmpty())
         {
-            NamingContext ctx = new NamingContext (_env, _name, _parent, _parser);
+            return shallowCopy();
-            ctx._bindings=_bindings;
-            return ctx;
         }
+
         if (cname.size() == 0)
             throw new NamingException ("Name is empty");
 
@@ -618,75 +541,12 @@ public class NamingContext implements Context, Cloneable, Dumpable
             if (binding == null)
                 throw new NameNotFoundException();
 
-            Object o = binding.getObject();
+            return dereference(binding.getObject(), cname.getPrefix(1).toString());
-
-            //handle links by looking up the link
-            if (o instanceof Reference)
-            {
-                //deference the object
-                try
-                {
-                    return NamingManager.getObjectInstance(o, cname.getPrefix(1), this, _env);
-                }
-                catch (NamingException e)
-                {
-                    throw e;
-                }
-                catch (Exception e)
-                {
-                    __log.warn("",e);
-                    throw new NamingException (e.getMessage());
-                }
-            }
-            else
-            {
-                //object is either a LinkRef which we don't dereference
-                //or a plain object in which case spec says we return it
-                return o;
-            }
         }
 
-
+        return getContext(cname).lookup (cname.getSuffix(1));
-        //it is a multipart name, recurse to the first subcontext
-        String firstComponent = cname.get(0);
-        Object ctx = null;
-
-        if (firstComponent.equals(""))
-            ctx = this;
-        else
-        {
-            Binding binding = getBinding (firstComponent);
-            if (binding == null)
-                throw new NameNotFoundException ();
-
-            ctx = binding.getObject();
-
-            if (ctx instanceof Reference)
-            {
-                //deference the object
-                try
-                {
-                    ctx = NamingManager.getObjectInstance(ctx, getNameParser("").parse(firstComponent), this, _env);
-                }
-                catch (NamingException e)
-                {
-                    throw e;
-                }
-                catch (Exception e)
-                {
-                    __log.warn("",e);
-                    throw new NamingException (e.getMessage());
-                }
-            }
-        }
-
-        if (!(ctx instanceof Context))
-            throw new NotContextException();
-
-        return ((Context)ctx).lookup (cname.getSuffix(1));
     }
 
-
     /*------------------------------------------------*/
     /**
      * Lookup link bound to name
@@ -702,7 +562,6 @@ public class NamingContext implements Context, Cloneable, Dumpable
         return lookupLink (_parser.parse(name));
     }
 
-
     /*------------------------------------------------*/
     /**
      * List all names bound at Context named by Name
@@ -718,62 +577,20 @@ public class NamingContext implements Context, Cloneable, Dumpable
         if(__log.isDebugEnabled())__log.debug("list() on Context="+getName()+" for name="+name);
         Name cname = toCanonicalName(name);
 
-
-
         if (cname == null)
         {
             return new NameEnumeration(__empty.iterator());
         }
 
-
         if (cname.size() == 0)
         {
            return new NameEnumeration (_bindings.values().iterator());
         }
 
-
-
         //multipart name
-        String firstComponent = cname.get(0);
+        return getContext(cname).list (cname.getSuffix(1));
-        Object ctx = null;
-
-        if (firstComponent.equals(""))
-            ctx = this;
-        else
-        {
-            Binding binding = getBinding (firstComponent);
-            if (binding == null)
-                throw new NameNotFoundException ();
-
-            ctx = binding.getObject();
-
-            if (ctx instanceof Reference)
-            {
-                //deference the object
-                if(__log.isDebugEnabled())__log.debug("Dereferencing Reference for "+name.get(0));
-                try
-                {
-                    ctx = NamingManager.getObjectInstance(ctx, getNameParser("").parse(firstComponent), this, _env);
-                }
-                catch (NamingException e)
-                {
-                    throw e;
-                }
-                catch (Exception e)
-                {
-                    __log.warn("",e);
-                    throw new NamingException (e.getMessage());
-                }
-            }
-        }
-
-        if (!(ctx instanceof Context))
-            throw new NotContextException();
-
-        return ((Context)ctx).list (cname.getSuffix(1));
     }
 
-
     /*------------------------------------------------*/
     /**
      * List all names bound at Context named by Name
@@ -789,8 +606,6 @@ public class NamingContext implements Context, Cloneable, Dumpable
         return list(_parser.parse(name));
     }
 
-
-
     /*------------------------------------------------*/
     /**
      * List all Bindings present at Context named by Name
@@ -815,48 +630,7 @@ public class NamingContext implements Context, Cloneable, Dumpable
            return new BindingEnumeration (_bindings.values().iterator());
         }
 
-
+        return getContext(cname).listBindings (cname.getSuffix(1));
-
-        //multipart name
-        String firstComponent = cname.get(0);
-        Object ctx = null;
-
-        //if a name has a leading "/" it is parsed as "" so ignore it by staying
-        //at this level in the tree
-        if (firstComponent.equals(""))
-            ctx = this;
-        else
-        {
-            //it is a non-empty name component
-            Binding binding = getBinding (firstComponent);
-            if (binding == null)
-                throw new NameNotFoundException ();
-
-            ctx = binding.getObject();
-
-            if (ctx instanceof Reference)
-            {
-                //deference the object
-                try
-                {
-                    ctx = NamingManager.getObjectInstance(ctx, getNameParser("").parse(firstComponent), this, _env);
-                }
-                catch (NamingException e)
-                {
-                    throw e;
-                }
-                catch (Exception e)
-                {
-                    __log.warn("",e);
-                    throw new NamingException (e.getMessage());
-                }
-            }
-        }
-
-        if (!(ctx instanceof Context))
-            throw new NotContextException();
-
-        return ((Context)ctx).listBindings (cname.getSuffix(1));
     }
 
 
@@ -917,49 +691,7 @@ public class NamingContext implements Context, Cloneable, Dumpable
         }
         else
         {
-            //walk down the subcontext hierarchy
+            getContext(cname).rebind (cname.getSuffix(1), obj);
-            if(__log.isDebugEnabled())__log.debug("Checking for existing binding for name="+cname+" for first element of name="+cname.get(0));
-
-            String firstComponent = cname.get(0);
-            Object ctx = null;
-
-
-            if (firstComponent.equals(""))
-                ctx = this;
-            else
-            {
-                Binding  binding = getBinding (name.get(0));
-                if (binding == null)
-                    throw new NameNotFoundException (name.get(0)+ " is not bound");
-
-                ctx = binding.getObject();
-
-
-                if (ctx instanceof Reference)
-                {
-                    //deference the object
-                    try
-                    {
-                        ctx = NamingManager.getObjectInstance(ctx, getNameParser("").parse(firstComponent), this, _env);
-                    }
-                    catch (NamingException e)
-                    {
-                        throw e;
-                    }
-                    catch (Exception e)
-                    {
-                        __log.warn("",e);
-                        throw new NamingException (e.getMessage());
-                    }
-                }
-            }
-
-            if (ctx instanceof Context)
-            {
-                ((Context)ctx).rebind (cname.getSuffix(1), obj);
-            }
-            else
-                throw new NotContextException ("Object bound at "+firstComponent +" is not a Context");
         }
     }
 
@@ -1020,7 +752,6 @@ public class NamingContext implements Context, Cloneable, Dumpable
         if (cname.size() == 0)
             throw new NamingException ("Name is empty");
 
-
         //if no subcontexts, just unbind it
         if (cname.size() == 1)
         {
@@ -1028,48 +759,7 @@ public class NamingContext implements Context, Cloneable, Dumpable
         }
         else
         {
-            //walk down the subcontext hierarchy
+            getContext(cname).unbind (cname.getSuffix(1));
-            if(__log.isDebugEnabled())__log.debug("Checking for existing binding for name="+cname+" for first element of name="+cname.get(0));
-
-            String firstComponent = cname.get(0);
-            Object ctx = null;
-
-
-            if (firstComponent.equals(""))
-                ctx = this;
-            else
-            {
-                Binding  binding = getBinding (name.get(0));
-                if (binding == null)
-                    throw new NameNotFoundException (name.get(0)+ " is not bound");
-
-                ctx = binding.getObject();
-
-                if (ctx instanceof Reference)
-                {
-                    //deference the object
-                    try
-                    {
-                        ctx = NamingManager.getObjectInstance(ctx, getNameParser("").parse(firstComponent), this, _env);
-                    }
-                    catch (NamingException e)
-                    {
-                        throw e;
-                    }
-                    catch (Exception e)
-                    {
-                        __log.warn("",e);
-                        throw new NamingException (e.getMessage());
-                    }
-                }
-            }
-
-            if (ctx instanceof Context)
-            {
-                ((Context)ctx).unbind (cname.getSuffix(1));
-            }
-            else
-                throw new NotContextException ("Object bound at "+firstComponent +" is not a Context");
         }
     }
 
@@ -1277,7 +967,7 @@ public class NamingContext implements Context, Cloneable, Dumpable
     @Override
     public Hashtable getEnvironment ()
     {
-        return (Hashtable)_env.clone();
+        return _env;
     }
 
     /*------------------------------------------------*/
@@ -1307,8 +997,10 @@ public class NamingContext implements Context, Cloneable, Dumpable
 
         if (binding!=null)
         {
-            if (_bindings.containsKey(key)) {
+            if (_bindings.putIfAbsent(key, binding)!=null)
-                if(_supportDeepBinding) {
+            {
+                if(isDeepBindingSupported())
+                {
                     // quietly return (no exception)
                     // this is jndi spec breaking, but is added to support broken
                     // jndi users like openejb.
@@ -1316,7 +1008,6 @@ public class NamingContext implements Context, Cloneable, Dumpable
                 }
                 throw new NameAlreadyBoundException(name.toString());
             }
-            _bindings.put(key,binding);
         }
     }
 
@@ -1329,10 +1020,9 @@ public class NamingContext implements Context, Cloneable, Dumpable
      */
     public Binding getBinding (Name name)
     {
-        return (Binding) _bindings.get(name.toString());
+        return _bindings.get(name.toString());
     }
 
-
     /*------------------------------------------------*/
     /**
      * Get a name to object binding from this Context
@@ -1342,7 +1032,7 @@ public class NamingContext implements Context, Cloneable, Dumpable
      */
     public Binding getBinding (String name)
     {
-        return (Binding) _bindings.get(name);
+        return _bindings.get(name);
     }
 
     /*------------------------------------------------*/
@@ -1406,49 +1096,14 @@ public class NamingContext implements Context, Cloneable, Dumpable
     @Override
     public String dump()
     {
-        StringBuilder buf = new StringBuilder();
+        return Dumpable.dump(this);
-        try
-        {
-            dump(buf,"");
-        }
-        catch(Exception e)
-        {
-            __log.warn(e);
-        }
-        return buf.toString();
     }
 
-
     /* ------------------------------------------------------------ */
     @Override
     public void dump(Appendable out,String indent) throws IOException
     {
-        out.append(this.getClass().getSimpleName()).append("@").append(Long.toHexString(this.hashCode())).append("\n");
+        Dumpable.dumpObjects(out,indent,this, _bindings);
-        int size=_bindings.size();
-        int i=0;
-        for (Map.Entry<String,Binding> entry : ((Map<String,Binding>)_bindings).entrySet())
-        {
-            boolean last=++i==size;
-            out.append(indent).append(" +- ").append(entry.getKey()).append(": ");
-
-            Binding binding = entry.getValue();
-            Object value = binding.getObject();
-
-            if ("comp".equals(entry.getKey()) && value instanceof Reference && "org.eclipse.jetty.jndi.ContextFactory".equals(((Reference)value).getFactoryClassName()))
-            {
-                ContextFactory.dump(out,indent+(last?"    ":" |  "));
-            }
-            else if (value instanceof Dumpable)
-            {
-                ((Dumpable)value).dump(out,indent+(last?"    ":" |  "));
-            }
-            else
-            {
-                out.append(value.getClass().getSimpleName()).append("=");
-                out.append(String.valueOf(value).replace('\n','|').replace('\r','|'));
-                out.append("\n");
-            }
-        }
     }
 
     private Collection<Listener> findListeners()