Reuse remote naming context servant

Changed POA policy to allow user-defined IDs
Published policies factory method on SPI
Adapted BindingIterator to generate its own servant ID

yoko-core/src/main/java/org/apache/yoko/orb/CosNaming/tnaming2/BindingIteratorImpl.java

@@ -4,6 +4,7 @@
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.yoko.orb.CosNaming.tnaming2.NamingContextImpl.BoundObject;
 import org.apache.yoko.orb.spi.naming.RemoteAccess;
@@ -22,6 +23,8 @@ public final class BindingIteratorImpl extends LocalObject implements BindingIte
     private static final long serialVersionUID = 1L;
 
     private static final class Core extends BindingIteratorPOA {
+        private static final AtomicLong NEXT_ID = new AtomicLong();
+        private final long instanceId = NEXT_ID.getAndIncrement();
         // the iterator use to access the bindings
         private final Iterator<BoundObject> iterator;
 
@@ -33,6 +36,11 @@ private static final class Core extends BindingIteratorPOA {
         public Core(Collection<BoundObject> boundObjects) {
             this.iterator = (new ArrayList<BoundObject>(boundObjects)).iterator();
         }
+
+        private byte[] getServantId() {
+            return ("BindingIterator#" + instanceId).getBytes();
+        }
+
         /**
          * Return the next object in the iteration sequence.
          * @param b The BindingHolder used to return the next item. If we've
@@ -91,7 +99,7 @@ private static final class POAServant extends BindingIteratorPOA {
         public POAServant(POA poa, Core core) throws Exception {
             this.poa = poa;
             this.core = core;
-            poa.activate_object(this);
+            poa.activate_object_with_id(core.getServantId(), this);
         }
 
         /**

yoko-core/src/main/java/org/apache/yoko/orb/CosNaming/tnaming2/NamingContextBase.java

@@ -72,7 +72,7 @@ public NamingContextBase() throws Exception {
      *            must already be bound in the context tree.
      * @param obj The object to be bound.
      */
-    public void bind(NameComponent[] n, org.omg.CORBA.Object obj) 
+    public void bind(NameComponent[] n, org.omg.CORBA.Object obj)
             throws NotFound, CannotProceed, InvalidName, AlreadyBound {
         // perform various name validations
         validateName(n);
@@ -270,7 +270,7 @@ public org.omg.CORBA.Object resolve(NameComponent[] n) throws NotFound, CannotPr
             NamingContext context = resolveContext(n[0]);
             NameComponent[] subName = extractSubName(n);
 
-            // now pass this along to the next context for the real bind operation.
+            // now pass this along to the next context for the real resolve operation.
             return context.resolve(subName);
         } else {
             NameComponent name = n[0];
@@ -280,9 +280,9 @@ public org.omg.CORBA.Object resolve(NameComponent[] n) throws NotFound, CannotPr
                 // Object was not found
                 throw new NotFound(NotFoundReason.missing_node, n);
             }
-            if (obj instanceof Resolvable) { 
+            if (obj instanceof Resolvable) {
             	return ((Resolvable)obj).resolve();
-            } else { 
+            } else {
             	return obj;
             }
         }
@@ -578,7 +578,7 @@ protected synchronized NamingContext resolveContext(NameComponent name) throws N
             throw new NotFound(NotFoundReason.not_context, new NameComponent[]{name});
         }
 
-        // in theory, this is a naming context. Narrow it an return. Any
+        // in theory, this is a naming context. Narrow it and return. Any
         // errors just become a NotFound exception
         try {
             return NamingContextHelper.narrow(resolvedReference);

yoko-core/src/main/java/org/apache/yoko/orb/CosNaming/tnaming2/NamingContextImpl.java

@@ -1,9 +1,12 @@
 package org.apache.yoko.orb.CosNaming.tnaming2;
 
 import java.util.HashMap;
+import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.yoko.orb.spi.naming.RemoteAccess;
+import org.apache.yoko.orb.util.UnmodifiableEnumMap;
 import org.omg.CORBA.INTERNAL;
 import org.omg.CORBA.LocalObject;
 import org.omg.CORBA.NO_PERMISSION;
@@ -25,24 +28,44 @@
 import org.omg.CosNaming.NamingContextPackage.NotFound;
 import org.omg.PortableServer.POA;
 import org.omg.PortableServer.Servant;
+import org.omg.PortableServer.POAPackage.ObjectNotActive;
 
 public final class NamingContextImpl extends LocalObject implements NamingContextExt, RemotableObject {
     private static final long serialVersionUID = 1L;
 
+    private static final class ServantCreationLock {
+    }
+
     private static final class Core extends NamingContextBase {
-        // the bindings maintained by this context
+        private static final AtomicLong NEXT_ID = new AtomicLong();
+
+        /** Unique number for this core */
+        private final long instanceId = NEXT_ID.getAndIncrement();
+
+        /** the unique ids for this context's servants (one per remote access level) */
+        @SuppressWarnings("serial")
+        private final Map<RemoteAccess, String> servantIds = new UnmodifiableEnumMap<RemoteAccess, String>(RemoteAccess.class) {
+            public String computeValueFor(RemoteAccess key) {
+                return "NamingContext#" + instanceId + "$" + key;
+            }
+        };
+
+        /** the bindings maintained by this context */
         private final HashMap<BindingKey, BoundObject> bindings = new HashMap<BindingKey, BoundObject>();
-        // the root context object
+
+        /** the root context object */
         private final org.omg.CORBA.Object rootContext;
 
+        private Core(org.omg.CORBA.Object rootContext) throws Exception {
+            this.rootContext = rootContext;
+        }
+
         /**
-         * Construct a TransientNamingContext subcontext.
-         * @param orb The orb this context is associated with.
-         * @param poa The POA the root context is activated under.
-         * @param root The root context.
+         * Get the servant id to use for this context with the specified remote
+         * access level
          */
-        Core(org.omg.CORBA.Object root) throws Exception {
-            rootContext = root;
+        private byte[] getServantId(RemoteAccess access) {
+            return servantIds.get(access).getBytes();
         }
 
         // abstract methods part of the interface contract that the
@@ -204,6 +227,11 @@ public boolean equals(Object other) {
 
                 return (Objects.equals(name.id, otherKey.name.id) && Objects.equals(name.kind, otherKey.name.kind));
             }
+
+            @Override
+            public String toString() {
+                return "" + name;
+            }
         }
 
     }
@@ -224,11 +252,11 @@ static POAServant create(NamingContextImpl localContext, Core core, POA poa, Rem
         final NamingContextBase core;
         final POA poa;
 
-        protected POAServant(NamingContextImpl localContext, Core core, POA poa) throws Exception {
+        protected POAServant(NamingContextImpl localContext, Core core, POA poa, byte[] servantId) throws Exception {
             this.localContext = localContext;
             this.core = core;
             this.poa = poa;
-            poa.activate_object(this);
+            poa.activate_object_with_id(servantId, this);
         }
 
         abstract Servant convertLocalContextToRemoteContext(NamingContextImpl o) throws Exception;
@@ -268,9 +296,9 @@ public final void list(int how_many, BindingListHolder bl, BindingIteratorHolder
 
         private static final class ReadOnly extends POAServant {
             ReadOnly(NamingContextImpl localContext, Core core, POA poa) throws Exception {
-                super(localContext, core, poa);
+                super(localContext, core, poa, core.getServantId(RemoteAccess.readOnly));
             }
-            
+
             private SystemException newSystemException() {
                 return new NO_PERMISSION();
             }
@@ -303,7 +331,7 @@ public void destroy() {
 
         private static final class ReadWrite extends POAServant {
             ReadWrite(NamingContextImpl localContext, Core core, POA poa) throws Exception {
-                super(localContext, core, poa);
+                super(localContext, core, poa, core.getServantId(RemoteAccess.readWrite));
             }
 
             @Override
@@ -370,10 +398,18 @@ private BoundObject(NameComponent name, org.omg.CORBA.Object boundObject, Bindin
             this.boundObject = boundObject;
             this.type = type;
         }
+
+        @Override
+        public String toString() {
+            return name + "->" + boundObject;
+        }
     }
 
     private final Core core;
 
+    /** lock for servant creation */
+    private final Object servantCreationLock = new ServantCreationLock();
+
     public NamingContextImpl() throws Exception {
         core = new Core(this);
     }
@@ -434,7 +470,18 @@ public void destroy() throws NotEmpty {
 
     @Override
     public Servant getServant(POA poa, RemoteAccess remoteAccess) throws Exception {
-        return POAServant.create(this, core, poa, remoteAccess);
+        byte[] sid = core.getServantId(remoteAccess);
+
+        // synchronize around creation to avoid a race
+        synchronized (servantCreationLock) {
+            // check whether the servant needs to be created
+            try {
+                return poa.id_to_servant(sid);
+            } catch (ObjectNotActive expected) {
+                // guaranteed to be the unique creator-thread for this servant
+                return POAServant.create(this, core, poa, remoteAccess);
+            }
+        }
     }
 
     @Override

yoko-core/src/main/java/org/apache/yoko/orb/spi/naming/NameServiceInitializer.java

@@ -27,24 +27,40 @@
 public class NameServiceInitializer extends LocalObject implements ORBInitializer {
     /** The property name to use to initialize an ORB with this initializer. */
     public static final String NS_ORB_INIT_PROP = ORBInitializer.class.getName() + "Class." + NameServiceInitializer.class.getName();
-    /** 
-     * The name of this name service, as used with <code>corbaloc:</code> URLs 
+    /**
+     * The name of this name service, as used with <code>corbaloc:</code> URLs
      * and with calls to {@link ORB#resolve_initial_references(String)}.
      */
     public static final String SERVICE_NAME = "NameService";
 
     /**
      * The POA name this name service will use to activate contexts.
      * The name service will first try <code>rootPoa.find_POA()</code>
-     * to find the POA with this name. If that returns null, it will 
-     * call <code>rootPoa.create_POA()</code> to create the POA with 
+     * to find the POA with this name. If that returns null, it will
+     * call <code>rootPoa.create_POA()</code> to create the POA with
      * this name.
      */
     public static final String POA_NAME = "NameServicePOA";
 
-    /** 
+    /**
+     * The policies required for the NameService POA. Users providing
+     * the NameService POA must include these policies when creating
+     * the POA. If creation is to be left to this initializer, these
+     * policies will be used automatically.
+     * @param rootPOA the root POA for the ORB in use
+     * @return a new Policy array object, owned by the caller
+     */
+    public static final Policy[] createPOAPolicies(POA rootPOA) {
+        return new Policy[] {
+                rootPOA.create_lifespan_policy(LifespanPolicyValue.TRANSIENT),
+                rootPOA.create_id_assignment_policy(IdAssignmentPolicyValue.USER_ID),
+                rootPOA.create_servant_retention_policy(ServantRetentionPolicyValue.RETAIN)
+        };
+    }
+
+    /**
      * The ORB argument that specifies remote accessibility of this name service.
-     * The next argument must be one of these literal string values: 
+     * The next argument must be one of these literal string values:
      * <ul>
      *   <li><code>"</code>{@link #readOnly}<code>"</code></li>
      *   <li><code>"</code>{@link #readWrite}<code>"</code></li>
@@ -57,7 +73,7 @@ abstract static class BootLocatorImpl extends LocalObject implements BootLocator
     private static final long serialVersionUID = 1L;
 
     private RemoteAccess remoteAccess = readWrite;
-    
+
     @Override
     public void pre_init(ORBInitInfo info) {
         try {
@@ -81,7 +97,7 @@ public void pre_init(ORBInitInfo info) {
     @Override
     public void post_init(ORBInitInfo info) {
         try {
-            
+
             final POA rootPOA = (POA) info.resolve_initial_references("RootPOA");
             final NamingContextImpl local = (NamingContextImpl) info.resolve_initial_references("NameService");
             final String serviceName = getServiceName(info);
@@ -97,16 +113,16 @@ public void locate(byte[] oid, ObjectHolder obj, BooleanHolder add) throws NotFo
 
                     try {
                         rootPOA.the_POAManager().activate();
-                        
+
                         final POA nameServicePOA = findOrCreatePOA(rootPOA);
                         nameServicePOA.the_POAManager().activate();
 
                         final Servant nameServant = local.getServant(nameServicePOA, remoteAccess);
-                        
+
                         // return the context stub via the object holder
                         obj.value = nameServant._this_object();
                         // return true via the boolean holder
-                        // to tell the boot manager to re-use 
+                        // to tell the boot manager to re-use
                         // this result so we only get called once
                         add.value = true;
                     } catch (Exception e) {
@@ -118,11 +134,7 @@ private POA findOrCreatePOA(final POA rootPOA) throws AdapterAlreadyExists, Inva
                     try {
                         return rootPOA.find_POA(POA_NAME, true);
                     } catch (AdapterNonExistent e) {
-                        final Policy[] policies = {
-                                rootPOA.create_lifespan_policy(LifespanPolicyValue.TRANSIENT),
-                                rootPOA.create_id_assignment_policy(IdAssignmentPolicyValue.SYSTEM_ID),
-                                rootPOA.create_servant_retention_policy(ServantRetentionPolicyValue.RETAIN)
-                        };
+                        final Policy[] policies = createPOAPolicies(rootPOA);
                         return rootPOA.create_POA(POA_NAME, null, policies);
                     }
                 }

yoko-core/src/main/java/org/apache/yoko/orb/util/UnmodifiableEnumMap.java

@@ -0,0 +1,45 @@
+package org.apache.yoko.orb.util;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.Map;
+import java.util.Set;
+
+public abstract class UnmodifiableEnumMap<K extends Enum<K>, V> extends EnumMap<K, V> {
+    private static final long serialVersionUID = 1L;
+
+    public UnmodifiableEnumMap(Class<K> keyType) {
+        super(keyType);
+        // initialise all values up front to avoid races later
+        for(K key : keyType.getEnumConstants())
+            super.put(key, computeValueFor(key));
+    }
+
+    protected abstract V computeValueFor(K key);
+
+    @Override
+    public final V remove(Object key) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public final V put(K key, V value) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public final void putAll(Map<? extends K, ? extends V> m) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public final Set<K> keySet() {
+        return Collections.unmodifiableSet(super.keySet());
+    }
+
+    @Override
+    public final Collection<V> values() {
+        return Collections.unmodifiableCollection(super.values());
+    }
+}