You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by David Jencks <da...@yahoo.com> on 2005/03/12 02:20:17 UTC

Re: Review - svn commit: r126264

I started looking through this also -- it looks very nice.  However, I  
have an ulterior motive :-)  If I understand the stub/skeleton  
generation code correctly, it does not currently handle overloaded  
methods properly, it just assumes the IDL operation name is the same as  
the java method name.  Do you have just sitting there waiting to submit  
or are you planning to write soon code to translate java names to IDL  
operation names taking account of overloaded methods?

Many thanks,
david jencks


On Jan 24, 2005, at 7:28 PM, Mark wrote:

> Thanks for the review.  I will go through your comments one-by-one  
> over the next few days.
>
> A few things to keep in mind:
>
> 1. The rmi/iiop code is fairly solid (with my fingers crossed).  I  
> hope to get in a bunch of test cases that following the Geronimo  
> design.
> 2. The stub/skeleton generation is farily *dirty* it.  You can  
> convience it to work, code generation style/speed were not my primary  
> objective.  The rmi-iiop was.  So I fully anticipate this to change in  
> the near future.
> 3. The original code was stand alone, hence there are a few other  
> interop.* packages.  Once I learn more about Geronimo, I anticipate a  
> much neater/tighter integration.  Of course, I think I will have lots  
> of help from Alan.  :-)
> 4. I don't mind making the code or style changes to conform tothe  
> Apache project.  The current style is a required habit for other  
> projects that I work on.  :-)
>
> I've added a few quick answers below see MD>>
>
> ( I only made it 1/2 way through this email... )
>
> Expect that I will be submitting lots more changes to Alan over the  
> comming days.
>
> Thanks
> Mark
>
> Dain Sundstrom wrote:
>
>> Mark,
>>
>> Wow!  This is very impressive work. I spent some time reading over  
>> this  (well the first half... the thing is huge).  Most of the  
>> comments, I  have are just my curiosity and don't really need to be  
>> fixed.  Other  are notes on differences between this code an  
>> geronimo, and I these  cases I think we should change current  
>> geronimo or this new code so we  don't have multiple ways to do the  
>> same thing (BTW, I generally don't  have a preference on which code  
>> we change unless one is broken :)  Then  there are the few coding  
>> convention things, and finally there are a few  nit picky things :)
>>
>> Thanks for the great work,
>>
>> -dain
>>
>>
>> On Jan 23, 2005, at 11:33 PM, adc@apache.org wrote:
>>
>>> Added: geronimo/trunk/modules/interop/src/idl/CosNaming.idl
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> idl/ CosNaming.idl?view=auto&rev=126264
>>>
>>> Added: geronimo/trunk/modules/interop/src/idl/GIOP.idl
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> idl/ GIOP.idl?view=auto&rev=126264
>>>
>>> Added: geronimo/trunk/modules/interop/src/idl/IIOP.idl
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> idl/ IIOP.idl?view=auto&rev=126264
>>>
>>> Added: geronimo/trunk/modules/interop/src/idl/IOP.idl
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> idl/ IOP.idl?view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/idl/org-apache-geronimo-interop-  
>>> rmi-iiop.idl
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> idl/ org-apache-geronimo-interop-rmi-iiop.idl?view=auto&rev=126264
>>
>>
>> The idl files need Apache license headers... maybe in the comments  
>> that  are copied into the generated code.
>
>
> MD>> If these are taken from the www.omg.org website, should we  
> include the apache headers?
>
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> CheckedException.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/CheckedException.java? 
>>> view=auto&rev=126264
>>>
>>> +/**
>>> + * * A wrapper that allows checked exceptions to be thrown as   
>>> unchecked.
>>> + */
>>> +public class CheckedException extends RuntimeException {
>>> +    public CheckedException(Throwable cause) {
>>> +        super(cause);
>>> +    }
>>> +}
>>
>>
>> How is this used?  Normally in Geronimo we have our invocation chains  
>>  (interceptor stacks) throw Throwable, and we just let exceptions  
>> flow  up.  In other cases, we divide exceptions into System and  
>> Application.   We let the System exceptions flow up and the  
>> Application exceptions are  wrapped in the results object since they  
>> are treated as a normal return  for ejbs.  Anyway, I'm just curious.   
>> If it falls into one of the basic  scenarios we already have a  
>> preferred solution for, I'd like to either  switch the exiting code  
>> to follow the new pattern or switch the new  iiop code to follow the  
>> current pattern.
>
>
> MD>> Its most likely that this class will go away soon.
>
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> CosNaming/iiop_stubs/NamingContext_Stub.1.txt
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/CosNaming/iiop_stubs/  
>>> NamingContext_Stub.1.txt?view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> CosNaming/iiop_stubs/NamingContext_Stub.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/CosNaming/iiop_stubs/  
>>> NamingContext_Stub.java?view=auto&rev=126264
>>
>>
>> Are these generated stubs based on the IDL above?  Normally, we don't  
>>  check in generated code, but I bet these almost never change.
>
>
> MD>> The name service stubs/skels are generated.  Right now, due to a  
> couple of issues, I have hand modified them and simply checked them  
> in.  This will need to change.  All other generated files go under  
> interop/target/src
>
>
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> InteropGBean.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/InteropGBean.java?view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> --- (empty file)
>>> +++   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> InteropGBean.java    Sun Jan 23 23:33:10 2005
>>>
>>> +/**
>>> + * A GBean that provides an example interop
>>> + *
>>> + * @version $Rev: $ $Date: $
>>> + */
>>> +public class InteropGBean implements GBeanLifecycle {
>>
>>
>> Is this test code?  If it is an example we want to ship with the   
>> server, maybe we should put it in an example package or rename it   
>> ExampleInteropGBean.
>
>
> MD>> This GBean was the interop/corba container loader.  maybe a  
> better name is in order.  I haven't had a chance to flesh out the  
> details for the Geronimo integration.  I'd like to see the interop  
> container be loaded by the InteropGBean.  The InteropGBean could be a  
> seperate plan or as part of the j2ee-server plan.  For the J2EE 1.4  
> configuration, the interop container would be enabled, but if a  
> customer didn't care about the interop container they could remove the  
> plan from the server configuration.
>
>>
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> SystemException.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/SystemException.java? 
>>> view=auto&rev=126264
>>
>>
>> Same comment as above for CheckedException.
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> adapter/Adapter.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/adapter/Adapter.java? 
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class Adapter {
>>> +    //
>>> +    // Public Accessible Properties
>>> +    //
>>
>>
>> I don't think we need comments like this... it is self explanatory.
>
>
> MD>>  Default template code .. Comments can be taken out. :-)
>
>>
>>
>>> +    protected String _bindName;
>>> +    protected String _remoteClassName;
>>> +    protected String _remoteInterfaceName;
>>> +    protected Vector _idVector;
>>> +    protected boolean _shared;
>>> +    protected ClassLoader _cl;
>>> +    protected RemoteInterface _ri;
>>
>>
>> Our naming convention doesn't use lead underscores.  Also we tend not  
>>  to use protected fields.  Maybe there is a subclass or some reason.
>
>
> MD>> I have noticed that.  Will be changed.
>
>>
>>> +    //
>>> +    // Internal Properrties
>>> +    //
>>> +
>>> +    protected Object _sharedObject;
>>> +    protected HashMap _objects;
>>> +    protected Class _remoteClassClass;
>>> +    protected Class _remoteInterfaceClass;
>>> +
>>> +    public Adapter() {
>>> +        _objects = new HashMap();
>>> +        _idVector = new Vector();
>>> +    }
>>> +
>>> +    /*
>>> +     * BindName is the name that will be registered with the INS   
>>> (Inter-operable Name Service)
>>> +     */
>>> +    public String getBindName() {
>>> +        return _bindName;
>>> +    }
>>> +
>>> +    public void setBindName(String bindName) {
>>> +        _bindName = bindName;
>>> +    }
>>> +
>>> +    /*
>>> +     * Is this a shared component?  If so this will invoke the   
>>> getInstance method on
>>> +     * the component ...
>>> +     */
>>> +    public boolean isShared() {
>>> +        return _shared;
>>> +    }
>>> +
>>> +    public void setShared(boolean shared) {
>>> +        _shared = shared;
>>> +    }
>>> +
>>> +    /*
>>> +     * The classloader that will load any dependancies of the  
>>> adapter  or corba skel interfaces.
>>> +     * Its should be set by the ejb container
>>> +     */
>>> +    public ClassLoader getClassLoader() {
>>> +        return _cl;
>>> +    }
>>> +
>>> +    public void setClassLoader(ClassLoader cl) {
>>> +        _cl = cl;
>>> +    }
>>
>>
>> Can we change these to be constructor injected and make the fields   
>> final?
>
> MD>> Its possible, I have to discuss this more with Alan .
>
>>
>>> +    /*
>>> +     * This is the name of the remote class that implements the   
>>> remote interface.
>>> +     *
>>> +     * This is only used if this adapter is going to directly  
>>> invoke  an object.  For the
>>> +     * EJB Container, the adapter will pass through the method   
>>> invocations.
>>> +     */
>>> +    public String getRemoteClassName() {
>>> +        return _remoteClassName;
>>> +    }
>>> +
>>> +    public void setRemoteClassName(String rcName) {
>>> +        _remoteClassName = rcName;
>>> +    }
>>> +
>>> +    /*
>>> +     * The remote interface name for the remote object.  This will   
>>> most likely be the name
>>> +     * of the EJB's RemoteInterface and RemoteHomeInterface
>>> +     *
>>> +     * The stub/skel generator will use this interface name.
>>> +     */
>>> +    public String getRemoteInterfaceName() {
>>> +        return _remoteInterfaceName;
>>> +    }
>>> +
>>> +    public void setRemoteInterfaceName(String riName) {
>>> +        _remoteInterfaceName = riName;
>>> +    }
>>> +
>>> +    /*
>>> +     * A list of public IDs that the remote object implements:
>>> +     *
>>> +     * IDL:....:1.0
>>> +     * RMI:....:X:Y
>>> +     */
>>> +    public Vector getIds() {
>>> +        return _idVector;
>>> +    }
>>> +
>>> +    public void addId(String id) {
>>> +        _idVector.add(id);
>>> +    }
>>> +
>>> +    public void removeId(String id) {
>>> +        _idVector.remove(id);
>>> +    }
>>> +
>>> +    /*
>>> +     * Return the skeleton implemention for the remote interface.    
>>> This interface has the
>>> +     * invoke method to handle the rmi/iiop messages.
>>> +     */
>>> +    public RemoteInterface getRemoteInterface() {
>>> +        if (_ri == null) {
>>> +            synchronized (this) {
>>> +                String riName = _remoteInterfaceName + "_Skeleton";
>>> +                _remoteInterfaceClass = loadClass(riName);
>>> +
>>> +                try {
>>> +                    _ri = (RemoteInterface)   
>>> _remoteInterfaceClass.newInstance();
>>> +                } catch (InstantiationException e) {
>>> +                    e.printStackTrace();  //To change body of catch  
>>>  statement use File | Settings | File Templates.
>>> +                } catch (IllegalAccessException e) {
>>> +                    e.printStackTrace();  //To change body of catch  
>>>  statement use File | Settings | File Templates.
>>> +                }
>>> +            }
>>> +        }
>>> +
>>> +        return _ri;
>>> +    }
>>
>>
>> This is double check locking.  The whole method needs to be   
>> synchronized.
>
> MD>> The double check code is wrong and I need to change it.    Does  
> the method require sync?  Maybe.  I recently came accross a strategy  
> for multiprocessor locking that uses a transient int as a memory  
> barrier.  Probably best if I don't attempt to explain it.  We can  
> optimize our locking.  The template that I came accross is as follows:
>
>    private volatile boolean _evaluated = false;
>
>    private Object _value;
>
>    /**
>     ** Sub-classes should override this method to get the object's  
> value
>     ** when it is first needed.
>     **/
>    public abstract Object evaluate();
>
>    public final Object getValue()
>    {
>        if (_volatileIsEffectiveMemoryBarrier)
>        {
>            if (! _evaluated)
>            {
>                synchronized (this)
>                {
>                    if (! _evaluated)
>                    {
>                        _value = evaluate();
>                        _evaluated = true;
>                    }
>                }
>            }
>        }
>        else
>        {
>            synchronized (this)
>            {
>                if (! _evaluated)
>                {
>                    _value = evaluate();
>                    _evaluated = true;
>                }
>            }
>        }
>        return _value;
>    }
>
>>
>>> +    /*
>>> +     * Get an object instance to invoke based on the object key.
>>> +     *
>>> +     * The objectKey could probably be passed to the EJB container  
>>> so  that the
>>> +     * container can directly invoke the ejb object as required.
>>> +     */
>>> +    public Object getInstance(byte[] objectKey) {
>>> +        String key = new String(objectKey);
>>> +        return getInstance(key);
>>> +    }
>>> +
>>> +    public Object getInstance(String key) {
>>> +        Object o = _objects.get(key);
>>> +
>>> +        if (o == null) {
>>> +            o = newInstance(key);
>>> +        }
>>> +
>>> +        return o;
>>> +    }
>>> +
>>> +    public Object newInstance(byte[] objectKey) {
>>> +        String key = new String(objectKey);
>>> +        return newInstance(key);
>>> +    }
>>> +
>>> +    public Object newInstance(String key) {
>>> +        Object o = null;
>>> +
>>> +        if (_remoteClassClass == null) {
>>> +            synchronized (this) {
>>> +                _remoteClassClass = loadClass(_remoteClassName);
>>> +            }
>>> +
>>> +            try {
>>> +                if (_shared) {
>>> +                    synchronized (this) {
>>> +                        Method m =   
>>> _remoteClassClass.getMethod("getInstance", (Class[]) null);
>>> +                        o = m.invoke(_remoteClassClass, (Object[])   
>>> null);
>>> +
>>> +                        if (o != null) {
>>> +                            _objects.put(key, o);
>>> +                        }
>>> +                    }
>>> +                } else {
>>> +                    o = _remoteClassClass.newInstance();
>>> +                    _objects.put(key, o);
>>> +                }
>>> +            } catch (InstantiationException e) {
>>> +                e.printStackTrace();  //To change body of catch   
>>> statement use File | Settings | File Templates.
>>> +            } catch (IllegalAccessException e) {
>>> +                e.printStackTrace();  //To change body of catch   
>>> statement use File | Settings | File Templates.
>>> +            } catch (NoSuchMethodException e) {
>>> +                e.printStackTrace();
>>> +            } catch (InvocationTargetException e) {
>>> +                e.printStackTrace();
>>> +            }
>>> +        }
>>> +
>>> +        return o;
>>> +    }
>>
>>
>> Again there is double check locking.  Also, if this is called in the   
>> critical path, I suggest we use CGLIB instead of reflection for   
>> construction as it is way faster.
>
> MD>>  I can look into CGLIB.
>
>>
>>> +    /*
>>> +     * Invoke method from the IIOP Message Handler.  The adapter is  
>>>  bound to the INS name service.
>>> +     * When an RMI/IIOP message is processed by the server, the   
>>> message handler will perform a lookup
>>> +     * on the name service to get the Adapter, then the invocation   
>>> will be passed to the adapter
>>> +     * The adapter will obtain the object key and then determine   
>>> which object instance to pass the
>>> +     * invocation to.
>>> +     */
>>
>>
>> This comment is missing the double star at the top to signify javadoc.
>>
>>> +    public void invoke(java.lang.String methodName, byte[]  
>>> objectKey,  org.apache.geronimo.interop.rmi.iiop.ObjectInputStream  
>>> input,  org.apache.geronimo.interop.rmi.iiop.ObjectOutputStream  
>>> output) {
>>> +        RemoteInterface skeleton = getRemoteInterface();
>>> +        Object instance = getInstance(objectKey);
>>> +
>>> +        if (instance != null) {
>>> +            skeleton.$invoke(methodName, objectKey, instance,  
>>> input,  output);
>>> +        } else {
>>> +            throw new org.omg.CORBA.OBJECT_NOT_EXIST(new   
>>> String(objectKey));
>>> +        }
>>> +    }
>>> +
>>> +    /*
>>> +     * Helper function to load a class.  This uses classloader for   
>>> the adapter.
>>> +     */
>>> +    protected Class loadClass(String name) {
>>> +        Class c = null;
>>> +
>>> +        try {
>>> +            c = _cl.loadClass(name);
>>> +        } catch (Exception e) {
>>> +            e.printStackTrace();
>>> +        }
>>> +
>>> +        return c;
>>> +    }
>>> +}
>>
>>
>> I think we should be at least logging the error or actually letting  
>> the  ClassNotFoundException through.  We also have a ClassLoading  
>> class  which you may want to use as it handles some of the more weird  
>> class  name formats.
>
> MD >> Will change with better Geronimo integration.
>
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> adapter/AdapterManager.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/adapter/AdapterManager.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> --- (empty file)
>>> +++   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> adapter/AdapterManager.java    Sun Jan 23 23:33:10 2005
>>> @@ -0,0 +1,43 @@
>>> +/**
>>> + *
>>> + *  Copyright 2004-2005 The Apache Software Foundation
>>> + *
>>> + *  Licensed under the Apache License, Version 2.0 (the "License");
>>> + *  you may not use this file except in compliance with the License.
>>> + *  You may obtain a copy of the License at
>>> + *
>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + *  Unless required by applicable law or agreed to in writing,   
>>> software
>>> + *  distributed under the License is distributed on an "AS IS"  
>>> BASIS,
>>> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or  
>>>  implied.
>>> + *
>>> + *  See the License for the specific language governing permissions  
>>>  and
>>> + *  limitations under the License.
>>> + */
>>> +package org.apache.geronimo.interop.adapter;
>>> +
>>> +import java.util.Hashtable;
>>> +
>>> +
>>> +public class AdapterManager {
>>> +    protected Hashtable _adapters;
>>> +    protected static AdapterManager _me = new AdapterManager();
>>
>>
>> Underscores and protected...
>
> MD>> Will do.
>
>>
>>> +    protected AdapterManager() {
>>> +        _adapters = new Hashtable();
>>> +    }
>>> +
>>> +    public static AdapterManager getInstance() {
>>> +        return _me;
>>> +    }
>>
>>
>> This is a bad idea.  Instead we should have a manger gbean and the   
>> singletonness would be handled by there being only one GBean instance  
>>  registered.
>
> MD>> Sure, this will change with the introduction of GBeans, better  
> integration...
>
>>
>>> +    public void registerAdapter(Adapter a) {
>>> +
>>> +        _adapters.put(a.getBindName(), a);
>>> +    }
>>> +
>>> +    public Adapter getAdapter(String objectName) {
>>> +        return (Adapter) _adapters.get(objectName);
>>> +    }
>>
>>
>> I prefer HashMap and synchronizing the methods that access the Map,  
>> but  this works in this case.
>
> MD>> Hashtables are going to be dead soon.  ;-)
>
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> client/InitialContextFactory.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/client/InitialContextFactory.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class InitialContextFactory
>>> +        implements javax.naming.spi.InitialContextFactory {
>>> +    private HashMap _startMap = new HashMap();
>>> +
>>> +    public Context getInitialContext(java.util.Hashtable env)  
>>> throws  NamingException {
>>> +        return   
>>> org.apache.geronimo.interop.rmi.iiop.client.ClientNamingContext.getIn 
>>> st ance(env);
>>> +    }
>>> +}
>>
>>
>> Is the _startMap variable used anywhere?
>
> MD>> I'd like to remove any naming in the interop and just use the  
> naming provided in Geronimo.
>
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> client/InitialContextFactoryBuilder.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/client/ 
>>> InitialContextFactoryBuilder.java? view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class InitialContextFactoryBuilder
>>> +        implements javax.naming.spi.InitialContextFactoryBuilder {
>>> +    public javax.naming.spi.InitialContextFactory   
>>> createInitialContextFactory(Hashtable env) {
>>> +        return new InitialContextFactory();
>>> +    }
>>> +}
>>
>>
>> What is this for?  Just curious...
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> generator/GenException.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/generator/GenException.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +/**
>>> + * User: Mark
>>> + * Date: Dec 21, 2004
>>> + * Time: 3:49:45 PM
>>> + */
>>
>>
>> Oops :)
>>
>>> +public class GenException
>>> +        extends Exception {
>>> +    public GenException() {
>>> +        super();
>>> +    }
>>> +
>>> +    public GenException(String message) {
>>> +        super(message);
>>> +    }
>>> +
>>> +    public GenException(Throwable cause) {
>>> +        super(cause);
>>> +    }
>>> +
>>> +    public GenException(String message, Throwable cause) {
>>> +        super(message, cause);
>>> +    }
>>> +}
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> generator/GenOptions.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/generator/GenOptions.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class GenOptions {
>>> +    protected String _genDir = "./";
>>> +    protected boolean _overwrite = false;
>>> +    protected boolean _verbose = false;
>>> +
>>> +    public GenOptions() {
>>> +    }
>>> +
>>> +    public GenOptions(String genDir, boolean overwrite, boolean   
>>> verbose) {
>>> +        _genDir = genDir;
>>> +        _overwrite = overwrite;
>>> +        _verbose = verbose;
>>> +    }
>>> +
>>> +    public String getGenDir() {
>>> +        return _genDir;
>>> +    }
>>> +
>>> +    public void setGenDir(String genDir) {
>>> +        _genDir = genDir;
>>> +    }
>>> +
>>> +    public boolean isOverwrite() {
>>> +        return _overwrite;
>>> +    }
>>> +
>>> +    public void setOverwrite(boolean overwrite) {
>>> +        _overwrite = overwrite;
>>> +    }
>>> +
>>> +    public boolean isVerbose() {
>>> +        return _verbose;
>>> +    }
>>> +
>>> +    public void setVerbose(boolean verbose) {
>>> +        _verbose = verbose;
>>> +    }
>>> +
>>> +}
>>
>>
>> Do we need both the constructor args and the setters?  If I set a   
>> variable is whatever is using this class going to adapt to the  
>> recently  changed variable?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> naming/NameService.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/naming/NameService.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> --- (empty file)
>>> +++   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> naming/NameService.java    Sun Jan 23 23:33:10 2005
>>> @@ -0,0 +1,66 @@
>>> +/**
>>> + *
>>> + *  Copyright 2004-2005 The Apache Software Foundation
>>> + *
>>> + *  Licensed under the Apache License, Version 2.0 (the "License");
>>> + *  you may not use this file except in compliance with the License.
>>> + *  You may obtain a copy of the License at
>>> + *
>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + *  Unless required by applicable law or agreed to in writing,   
>>> software
>>> + *  distributed under the License is distributed on an "AS IS"  
>>> BASIS,
>>> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or  
>>>  implied.
>>> + *
>>> + *  See the License for the specific language governing permissions  
>>>  and
>>> + *  limitations under the License.
>>> + */
>>> +package org.apache.geronimo.interop.naming;
>>> +
>>> +import java.util.HashMap;
>>> +import javax.naming.NamingException;
>>> +
>>> +import org.apache.geronimo.interop.adapter.Adapter;
>>> +
>>> +
>>> +public class NameService {
>>> +    protected static NameService _ns = null;
>>
>>
>> protected and underscore
>>
>>> +    public static NameService getInstance() {
>>> +        if (_ns == null) {
>>> +            synchronized (NameService.class) {
>>> +                if (_ns == null) {
>>> +                    _ns = new NameService();
>>> +                    _ns.init();
>>> +                }
>>> +            }
>>> +        }
>>> +
>>> +        return _ns;
>>> +    }
>>
>>
>> Double check locking
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> naming/NamingContext.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/naming/NamingContext.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> --- (empty file)
>>> +++   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> naming/NamingContext.java    Sun Jan 23 23:33:10 2005
>>> @@ -0,0 +1,150 @@
>>> +/**
>>> + *
>>> + *  Copyright 2004-2005 The Apache Software Foundation
>>> + *
>>> + *  Licensed under the Apache License, Version 2.0 (the "License");
>>> + *  you may not use this file except in compliance with the License.
>>> + *  You may obtain a copy of the License at
>>> + *
>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + *  Unless required by applicable law or agreed to in writing,   
>>> software
>>> + *  distributed under the License is distributed on an "AS IS"  
>>> BASIS,
>>> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or  
>>>  implied.
>>> + *
>>> + *  See the License for the specific language governing permissions  
>>>  and
>>> + *  limitations under the License.
>>> + */
>>> +package org.apache.geronimo.interop.naming;
>>> +
>>> +import java.util.HashMap;
>>> +import javax.naming.NameNotFoundException;
>>> +import javax.naming.NamingException;
>>> +
>>> +import org.apache.geronimo.interop.adapter.Adapter;
>>> +
>>> +
>>> +public class NamingContext {
>>> +    public static final NamingContext getInstance(Class baseClass) {
>>> +        NamingContext context;
>>> +        synchronized (_contextMap) {
>>> +            context = (NamingContext) _contextMap.get(baseClass);
>>> +            if (context == null) {
>>> +                context = new NamingContext();
>>> +                _contextMap.put(baseClass, context);
>>> +                context.init(baseClass);
>>> +            }
>>> +        }
>>> +        return context;
>>> +    }
>>> +
>>> +    private static ThreadLocal _current = new ThreadLocal();
>>
>>
>> Should this be an InheritableThreadLocal?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/BooleanProperty.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/BooleanProperty.java?  
>>> view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/ByteProperty.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/ByteProperty.java?  
>>> view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/DoubleProperty.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/DoubleProperty.java?  
>>> view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/FloatProperty.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/FloatProperty.java?  
>>> view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/IntProperty.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/IntProperty.java?  
>>> view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/LongProperty.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/LongProperty.java?  
>>> view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/ShortProperty.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/ShortProperty.java?  
>>> view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/StringProperty.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/StringProperty.java?  
>>> view=auto&rev=126264
>>
>>
>> How are these used?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> properties/SystemProperties.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/properties/SystemProperties.java?  
>>> view=auto&rev=126264
>>
>>
>> Is this a bridge to java.lang.System properties?  If so, do we really  
>>  want to support system properties?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> repository/Repository.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/repository/Repository.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class Repository {
>>> +    // ??
>>> +}
>>
>>
>> Say what? :)
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> rmi/iiop/IiopVersion.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/rmi/iiop/IiopVersion.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class IiopVersion {
>>> +}
>>
>>
>> What's this for?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> rmi/iiop/ListenerInfo.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/rmi/iiop/ListenerInfo.java?  
>>> view=auto&rev=126264
>>> +public class ListenerInfo {
>>> +    public int protocol;
>>> +
>>> +    public String host;
>>> +
>>> +    public int port;
>>> +}
>>
>>
>> Do these have to be public fields?  Normally we use, private fields   
>> with getters and setters.
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> rmi/iiop/NameServiceOperations_Skeleton.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/rmi/iiop/  
>>> NameServiceOperations_Skeleton.java?view=auto&rev=126264
>>
>>
>> Do we want the Skeletons checked in?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> rmi/iiop/ObjectKey.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/rmi/iiop/ObjectKey.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class ObjectKey {
>>> +    public static final int TYPE_MANAGER = 'M';
>>> +
>>> +    public static final int TYPE_SESSION = 'S';
>>> +
>>> +    public int type;
>>> +    public String username = "";
>>> +    public String password = "";
>>> +    public String component = "";
>>> +    public String sessionID = "";
>>
>>
>> public fields?
>>
>>> +    public void checkPassword() {
>>> +        User.getInstance(username).login(password);
>>> +    }
>>
>>
>> Shouldn't this go though Geronimo security?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> rmi/iiop/ObjectRef.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/rmi/iiop/ObjectRef.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +    public int $getIiopVersion() {
>>> +        return _iiopVersion;
>>> +    }
>>
>>
>> Why do all the methods in this class start with a dollar sign?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> rmi/iiop/client/ClientNamingContext.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/rmi/iiop/client/ 
>>> ClientNamingContext.java? view=auto&rev=126264
>>
>>
>> We should consider merging this with the Geronimo naming stuff... not  
>> a  task for today, but (much) later on :)
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> rmi/iiop/compiler/StubCompiler.txt
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/rmi/iiop/compiler/StubCompiler.txt?  
>>> view=auto&rev=126264
>>
>>
>> Why is this a text file?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> rmi/iiop/server/SocketListener.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/rmi/iiop/server/SocketListener.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class SocketListener extends Thread {
>>> +    public static SocketListener getInstance() {
>>> +        return new SocketListener();
>>> +    }
>>
>>
>> Why is there a static factory?
>>
>>> +    // private data
>>> +
>>> +    private String _name;
>>> +
>>> +    private String _host;
>>> +
>>> +    private int _port;
>>> +
>>> +    private int _listenBacklog;
>>> +
>>> +    private java.net.ServerSocket _listener;
>>> +
>>> +    // internal methods
>>> +
>>> +    protected void init() {
>>> +        _host = "localhost";
>>> +        _port = 2000;
>>> +        _listenBacklog = 10;
>>> +        setDaemon(true);
>>> +    }
>>> +
>>> +    // public methods
>>> +
>>> +    public void setHost(String host) {
>>> +        _host = host;
>>> +    }
>>> +
>>> +    public void setPort(int port) {
>>> +        _port = port;
>>> +    }
>>> +
>>> +    public void setListenBacklog(int backlog) {
>>> +        _listenBacklog = backlog;
>>> +    }
>>> +
>>> +    public void run() {
>>> +        String iiopURL = "iiop://" + _host + ":" + _port;
>>> +        ListenerInfo listenerInfo = new ListenerInfo();
>>> +        listenerInfo.protocol = Protocol.IIOP; // TODO: other   
>>> protocols (IIOPS etc.)
>>> +        listenerInfo.host = _host;
>>> +        listenerInfo.port = _port;
>>> +        try {
>>> +            InetAddress addr = InetAddress.getByName(_host);
>>> +            _listener = new java.net.ServerSocket(_port,   
>>> _listenBacklog, addr);
>>> +        } catch (Exception ex) {
>>> +            System.out.println("SocketListener: Error creating  
>>> server  socket.");
>>> +            ex.printStackTrace();
>>> +            try {
>>> +                Socket socket = new Socket(_host, _port);
>>> +                socket.close();
>>> +                System.out.println("SocketListener: Error server   
>>> already running: " + iiopURL);
>>> +                ex.printStackTrace();
>>> +            } catch (Exception ignore) {
>>> +            }
>>> +            return;
>>> +        }
>>> +        new CheckConnect().start();
>>> +        for (; ;) {
>>> +            java.net.Socket socket;
>>> +            try {
>>> +                socket = _listener.accept();
>>> +            } catch (Exception ex) {
>>> +                throw new SystemException(ex); // TODO: log error   
>>> message
>>> +            }
>>> +            MessageHandler.getInstance(listenerInfo,  
>>> socket).start();
>>> +        }
>>> +    }
>>> +
>>> +    private class CheckConnect extends Thread {
>>> +        public void run() {
>>> +            try {
>>> +                Socket socket = new Socket(_host, _port);
>>> +                socket.close();
>>> +                if (!SystemProperties.quiet()) {
>>> +                      
>>> System.out.println(formatAcceptingIiopConnections());
>>> +                }
>>> +            } catch (Exception ex) {
>>> +                warnConnectFailed(_host, _port);
>>> +            }
>>> +        }
>>> +    }
>>> +
>>> +    // format methods
>>> +
>>> +    protected String formatAcceptingIiopConnections() {
>>> +        String msg =  
>>> "SocketListener.formatAcceptingIiopConnection():  ";
>>> +        return msg;
>>> +    }
>>> +
>>> +    // log methods
>>> +
>>> +    protected void warnConnectFailed(String host, int port) {
>>> +        System.out.println("SocketListener.warnConnectFailed():  
>>> host  = " + host + ", port = " + port);
>>> +    }
>>> +}
>>
>>
>> I suggest, you either require all state data such as host and port be  
>>  passed in via the constructor, or make the setters throw an "already  
>>  running" exception.  Otherwise people mistakenly think the can  
>> change  the port.
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> security/Role.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/security/Role.java?view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> security/SimpleSubject.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/security/SimpleSubject.java?  
>>> view=auto&rev=126264
>>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> security/User.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/security/User.java?view=auto&rev=126264
>>
>>
>> What is the plan for these with regards to the Geronimo security  
>> module?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> util/SystemUtil.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/  
>>> org/apache/geronimo/interop/util/SystemUtil.java? 
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> +public class SystemUtil {
>>> +    // properties
>>> +
>>> +    public static final StringProperty vmVersionProperty =
>>> +            new StringProperty(SystemProperties.class,   
>>> "java.vm.version");
>>> +
>>> +    // private data
>>> +
>>> +    private static String _vmVersion =  
>>> vmVersionProperty.getString();
>>> +
>>> +    private static boolean _isJDK13 = _vmVersion.startsWith("1.3")
>>> +                                      ||   
>>> _vmVersion.startsWith("CrE-ME V4.00");
>>> +
>>> +    private static boolean _isJDK14 = _vmVersion.startsWith("1.4");
>>> +
>>> +    // public methods
>>> +
>>> +    public static String getExecutableSuffix() {
>>> +        return isWindows() ? ".exe" : "";
>>> +    }
>>> +
>>> +    public static String getShellScriptSuffix() {
>>> +        return isWindows() ? ".bat" : ".sh";
>>> +    }
>>> +
>>> +    public static boolean isJDK13() {
>>> +        return _isJDK13;
>>> +    }
>>> +
>>> +    public static boolean isJDK14() {
>>> +        return _isJDK14;
>>> +    }
>>> +
>>> +    public static boolean isWindows() {
>>> +        return java.io.File.separatorChar == '\\';
>>> +    }
>>> +}
>>
>>
>> How is this used?
>>
>>> Added:   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> util/ThreadContext.java
>>> Url:   
>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>> java/ org/apache/geronimo/interop/util/ThreadContext.java?  
>>> view=auto&rev=126264
>>> ===================================================================== 
>>> == =======
>>> --- (empty file)
>>> +++   
>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>> util/ThreadContext.java    Sun Jan 23 23:33:10 2005
>>> @@ -0,0 +1,135 @@
>>> +/**
>>> + *
>>> + *  Copyright 2004-2005 The Apache Software Foundation
>>> + *
>>> + *  Licensed under the Apache License, Version 2.0 (the "License");
>>> + *  you may not use this file except in compliance with the License.
>>> + *  You may obtain a copy of the License at
>>> + *
>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + *  Unless required by applicable law or agreed to in writing,   
>>> software
>>> + *  distributed under the License is distributed on an "AS IS"  
>>> BASIS,
>>> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or  
>>>  implied.
>>> + *
>>> + *  See the License for the specific language governing permissions  
>>>  and
>>> + *  limitations under the License.
>>> + */
>>> +package org.apache.geronimo.interop.util;
>>> +
>>> +import java.util.HashMap;
>>> +
>>> +import org.apache.geronimo.interop.SystemException;
>>> +
>>> +
>>> +public abstract class ThreadContext {
>>> +    private static HashMap _primTypes;
>>> +
>>> +    private static ThreadLocal _defaultRmiHost = new ThreadLocal();
>>> +
>>> +    private static ThreadLocal _defaultRmiPort = new ThreadLocal();
>>> +
>>> +    static {
>>> +        _primTypes = new HashMap();
>>> +        _primTypes.put("boolean", boolean.class);
>>> +        _primTypes.put("char", char.class);
>>> +        _primTypes.put("byte", byte.class);
>>> +        _primTypes.put("short", short.class);
>>> +        _primTypes.put("int", int.class);
>>> +        _primTypes.put("long", long.class);
>>> +        _primTypes.put("float", float.class);
>>> +        _primTypes.put("double", double.class);
>>> +        _primTypes.put("boolean[]", boolean[].class);
>>> +        _primTypes.put("char[]", char[].class);
>>> +        _primTypes.put("byte[]", byte[].class);
>>> +        _primTypes.put("short[]", short[].class);
>>> +        _primTypes.put("int[]", int[].class);
>>> +        _primTypes.put("long[]", long[].class);
>>> +        _primTypes.put("float[]", float[].class);
>>> +        _primTypes.put("double[]", double[].class);
>>> +    }
>>> +
>>> +    public static String getDefaultRmiHost() {
>>> +        String host = (String) _defaultRmiHost.get();
>>> +        if (host == null) {
>>> +            host = "0";
>>> +        }
>>> +        return host;
>>> +    }
>>> +
>>> +    public static int getDefaultRmiPort() {
>>> +        Integer port = (Integer) _defaultRmiPort.get();
>>> +        if (port == null) {
>>> +            port = IntegerCache.get(0);
>>> +        }
>>> +        return port.intValue();
>>> +    }
>>> +
>>> +    public static Class loadClass(String className) {
>>> +        Class t = (Class) _primTypes.get(className);
>>> +        if (t != null) {
>>> +            return t;
>>> +        }
>>> +        try {
>>> +            ClassLoader loader =   
>>> Thread.currentThread().getContextClassLoader();
>>> +            if (loader == null) {
>>> +                return Class.forName(className);
>>> +            } else {
>>> +                return loader.loadClass(className);
>>> +            }
>>> +        } catch (RuntimeException ex) {
>>> +            throw (RuntimeException) ex;
>>> +        } catch (Exception ex) {
>>> +            throw new SystemException(ex);
>>> +        }
>>> +    }
>>
>>
>> This is unlikely to work in Geronimo as we almost never set context   
>> class loader.  Normally we try to explicitly pass around the   
>> classloader.
>>
>>> +
>>> +    public static Class loadClass(String className, Class   
>>> parentClass) {
>>> +        if (parentClass == null) {
>>> +            return loadClass(className);
>>> +        }
>>> +        Class t = (Class) _primTypes.get(className);
>>> +        if (t != null) {
>>> +            return t;
>>> +        }
>>> +        try {
>>> +            ClassLoader loader = parentClass.getClassLoader();
>>> +            if (loader == null) {
>>> +                return loadClass(className);
>>> +            } else {
>>> +                return loader.loadClass(className);
>>> +            }
>>> +        } catch (RuntimeException ex) {
>>> +            throw (RuntimeException) ex;
>>> +        } catch (Exception ex) {
>>> +            throw new SystemException(ex);
>>> +        }
>>> +    }
>>> +
>>> +    public static Class loadClassOrReturnNullIfNotFound(String   
>>> className) {
>>> +        try {
>>> +            return loadClass(className);
>>> +        } catch (RuntimeException ex) {
>>> +            return null;
>>> +        }
>>> +    }
>>> +
>>> +    public static Class loadClassOrReturnNullIfNotFound(String   
>>> className, Class parentClass) {
>>> +        if (parentClass == null) {
>>> +            return loadClassOrReturnNullIfNotFound(className);
>>> +        }
>>> +        try {
>>> +            return loadClass(className, parentClass);
>>> +        } catch (RuntimeException ex) {
>>> +            return null;
>>> +        }
>>> +    }
>>
>>
>> Most of this class loader stuff is handled by the class ClassLoading   
>> which handles some of the more weird stuff.
>>
>>
>> .
>>
>


Re: Review - svn commit: r126264

Posted by Dain Sundstrom <ds...@gluecode.com>.
On Mar 14, 2005, at 7:07 AM, Mark wrote:

> Many appologies - I just haven't had the time to clean up the code on 
> my laptop with all my changes and then get them submitted for a commit 
> by somebody else.  I have a few outstanding items to resolve before 
> submitting the code to check in.  I know that you have been busy 
> working towards CTS certification and since the interop code won't be 
> used for the certification, I have lowered the priority on my code. 
> :-(  However, ...
>
> Yes, my code properly handles the method name overload as per the 
> RMI-IIOP spec.
>
> Hopefully within the next few days, I will have a new zip file with 
> all my changes so they can be committed.

Can you create a diff instead of a zip?  Diffs are much easier to deal 
with.

My guess is you already know this, but I'll go over it again for the 
lurkers on the list.  Creating a diff with subversion is very easy.  
You can add and remove files with 'svn add' and 'svn remove' 
respectively.  Then just run 'svn diff' from the root directory, and 
pipe the result to a file.  Post this in a JIRA issue, and one of the 
committers will look it over.

-dain


Re: Review - svn commit: r126264

Posted by Mark <de...@hotmail.com>.
Many appologies - I just haven't had the time to clean up the code on my 
laptop with all my changes and then get them submitted for a commit by 
somebody else.  I have a few outstanding items to resolve before 
submitting the code to check in.  I know that you have been busy working 
towards CTS certification and since the interop code won't be used for 
the certification, I have lowered the priority on my code. :-(  However, ...

Yes, my code properly handles the method name overload as per the 
RMI-IIOP spec.

Hopefully within the next few days, I will have a new zip file with all 
my changes so they can be committed.

Thanks
Mark

David Jencks wrote:

> I started looking through this also -- it looks very nice.  However, 
> I  have an ulterior motive :-)  If I understand the stub/skeleton  
> generation code correctly, it does not currently handle overloaded  
> methods properly, it just assumes the IDL operation name is the same 
> as  the java method name.  Do you have just sitting there waiting to 
> submit  or are you planning to write soon code to translate java names 
> to IDL  operation names taking account of overloaded methods?
>
> Many thanks,
> david jencks
>
>
> On Jan 24, 2005, at 7:28 PM, Mark wrote:
>
>> Thanks for the review.  I will go through your comments one-by-one  
>> over the next few days.
>>
>> A few things to keep in mind:
>>
>> 1. The rmi/iiop code is fairly solid (with my fingers crossed).  I  
>> hope to get in a bunch of test cases that following the Geronimo  
>> design.
>> 2. The stub/skeleton generation is farily *dirty* it.  You can  
>> convience it to work, code generation style/speed were not my 
>> primary  objective.  The rmi-iiop was.  So I fully anticipate this to 
>> change in  the near future.
>> 3. The original code was stand alone, hence there are a few other  
>> interop.* packages.  Once I learn more about Geronimo, I anticipate 
>> a  much neater/tighter integration.  Of course, I think I will have 
>> lots  of help from Alan.  :-)
>> 4. I don't mind making the code or style changes to conform tothe  
>> Apache project.  The current style is a required habit for other  
>> projects that I work on.  :-)
>>
>> I've added a few quick answers below see MD>>
>>
>> ( I only made it 1/2 way through this email... )
>>
>> Expect that I will be submitting lots more changes to Alan over the  
>> comming days.
>>
>> Thanks
>> Mark
>>
>> Dain Sundstrom wrote:
>>
>>> Mark,
>>>
>>> Wow!  This is very impressive work. I spent some time reading over  
>>> this  (well the first half... the thing is huge).  Most of the  
>>> comments, I  have are just my curiosity and don't really need to be  
>>> fixed.  Other  are notes on differences between this code an  
>>> geronimo, and I these  cases I think we should change current  
>>> geronimo or this new code so we  don't have multiple ways to do the  
>>> same thing (BTW, I generally don't  have a preference on which code  
>>> we change unless one is broken :)  Then  there are the few coding  
>>> convention things, and finally there are a few  nit picky things :)
>>>
>>> Thanks for the great work,
>>>
>>> -dain
>>>
>>>
>>> On Jan 23, 2005, at 11:33 PM, adc@apache.org wrote:
>>>
>>>> Added: geronimo/trunk/modules/interop/src/idl/CosNaming.idl
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> idl/ CosNaming.idl?view=auto&rev=126264
>>>>
>>>> Added: geronimo/trunk/modules/interop/src/idl/GIOP.idl
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> idl/ GIOP.idl?view=auto&rev=126264
>>>>
>>>> Added: geronimo/trunk/modules/interop/src/idl/IIOP.idl
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> idl/ IIOP.idl?view=auto&rev=126264
>>>>
>>>> Added: geronimo/trunk/modules/interop/src/idl/IOP.idl
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> idl/ IOP.idl?view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/idl/org-apache-geronimo-interop-  
>>>> rmi-iiop.idl
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> idl/ org-apache-geronimo-interop-rmi-iiop.idl?view=auto&rev=126264
>>>
>>>
>>>
>>> The idl files need Apache license headers... maybe in the comments  
>>> that  are copied into the generated code.
>>
>>
>>
>> MD>> If these are taken from the www.omg.org website, should we  
>> include the apache headers?
>>
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> CheckedException.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  org/apache/geronimo/interop/CheckedException.java? 
>>>> view=auto&rev=126264
>>>>
>>>> +/**
>>>> + * * A wrapper that allows checked exceptions to be thrown as   
>>>> unchecked.
>>>> + */
>>>> +public class CheckedException extends RuntimeException {
>>>> +    public CheckedException(Throwable cause) {
>>>> +        super(cause);
>>>> +    }
>>>> +}
>>>
>>>
>>>
>>> How is this used?  Normally in Geronimo we have our invocation 
>>> chains   (interceptor stacks) throw Throwable, and we just let 
>>> exceptions  flow  up.  In other cases, we divide exceptions into 
>>> System and  Application.   We let the System exceptions flow up and 
>>> the  Application exceptions are  wrapped in the results object since 
>>> they  are treated as a normal return  for ejbs.  Anyway, I'm just 
>>> curious.   If it falls into one of the basic  scenarios we already 
>>> have a  preferred solution for, I'd like to either  switch the 
>>> exiting code  to follow the new pattern or switch the new  iiop code 
>>> to follow the  current pattern.
>>
>>
>>
>> MD>> Its most likely that this class will go away soon.
>>
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> CosNaming/iiop_stubs/NamingContext_Stub.1.txt
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/CosNaming/iiop_stubs/  
>>>> NamingContext_Stub.1.txt?view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> CosNaming/iiop_stubs/NamingContext_Stub.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/CosNaming/iiop_stubs/  
>>>> NamingContext_Stub.java?view=auto&rev=126264
>>>
>>>
>>>
>>> Are these generated stubs based on the IDL above?  Normally, we 
>>> don't   check in generated code, but I bet these almost never change.
>>
>>
>>
>> MD>> The name service stubs/skels are generated.  Right now, due to 
>> a  couple of issues, I have hand modified them and simply checked 
>> them  in.  This will need to change.  All other generated files go 
>> under  interop/target/src
>>
>>
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> InteropGBean.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  
>>>> org/apache/geronimo/interop/InteropGBean.java?view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> --- (empty file)
>>>> +++   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> InteropGBean.java    Sun Jan 23 23:33:10 2005
>>>>
>>>> +/**
>>>> + * A GBean that provides an example interop
>>>> + *
>>>> + * @version $Rev: $ $Date: $
>>>> + */
>>>> +public class InteropGBean implements GBeanLifecycle {
>>>
>>>
>>>
>>> Is this test code?  If it is an example we want to ship with the   
>>> server, maybe we should put it in an example package or rename it   
>>> ExampleInteropGBean.
>>
>>
>>
>> MD>> This GBean was the interop/corba container loader.  maybe a  
>> better name is in order.  I haven't had a chance to flesh out the  
>> details for the Geronimo integration.  I'd like to see the interop  
>> container be loaded by the InteropGBean.  The InteropGBean could be 
>> a  seperate plan or as part of the j2ee-server plan.  For the J2EE 
>> 1.4  configuration, the interop container would be enabled, but if a  
>> customer didn't care about the interop container they could remove 
>> the  plan from the server configuration.
>>
>>>
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> SystemException.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  org/apache/geronimo/interop/SystemException.java? 
>>>> view=auto&rev=126264
>>>
>>>
>>>
>>> Same comment as above for CheckedException.
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> adapter/Adapter.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  org/apache/geronimo/interop/adapter/Adapter.java? 
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class Adapter {
>>>> +    //
>>>> +    // Public Accessible Properties
>>>> +    //
>>>
>>>
>>>
>>> I don't think we need comments like this... it is self explanatory.
>>
>>
>>
>> MD>>  Default template code .. Comments can be taken out. :-)
>>
>>>
>>>
>>>> +    protected String _bindName;
>>>> +    protected String _remoteClassName;
>>>> +    protected String _remoteInterfaceName;
>>>> +    protected Vector _idVector;
>>>> +    protected boolean _shared;
>>>> +    protected ClassLoader _cl;
>>>> +    protected RemoteInterface _ri;
>>>
>>>
>>>
>>> Our naming convention doesn't use lead underscores.  Also we tend 
>>> not   to use protected fields.  Maybe there is a subclass or some 
>>> reason.
>>
>>
>>
>> MD>> I have noticed that.  Will be changed.
>>
>>>
>>>> +    //
>>>> +    // Internal Properrties
>>>> +    //
>>>> +
>>>> +    protected Object _sharedObject;
>>>> +    protected HashMap _objects;
>>>> +    protected Class _remoteClassClass;
>>>> +    protected Class _remoteInterfaceClass;
>>>> +
>>>> +    public Adapter() {
>>>> +        _objects = new HashMap();
>>>> +        _idVector = new Vector();
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * BindName is the name that will be registered with the INS   
>>>> (Inter-operable Name Service)
>>>> +     */
>>>> +    public String getBindName() {
>>>> +        return _bindName;
>>>> +    }
>>>> +
>>>> +    public void setBindName(String bindName) {
>>>> +        _bindName = bindName;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * Is this a shared component?  If so this will invoke the   
>>>> getInstance method on
>>>> +     * the component ...
>>>> +     */
>>>> +    public boolean isShared() {
>>>> +        return _shared;
>>>> +    }
>>>> +
>>>> +    public void setShared(boolean shared) {
>>>> +        _shared = shared;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * The classloader that will load any dependancies of the  
>>>> adapter  or corba skel interfaces.
>>>> +     * Its should be set by the ejb container
>>>> +     */
>>>> +    public ClassLoader getClassLoader() {
>>>> +        return _cl;
>>>> +    }
>>>> +
>>>> +    public void setClassLoader(ClassLoader cl) {
>>>> +        _cl = cl;
>>>> +    }
>>>
>>>
>>>
>>> Can we change these to be constructor injected and make the fields   
>>> final?
>>
>>
>> MD>> Its possible, I have to discuss this more with Alan .
>>
>>>
>>>> +    /*
>>>> +     * This is the name of the remote class that implements the   
>>>> remote interface.
>>>> +     *
>>>> +     * This is only used if this adapter is going to directly  
>>>> invoke  an object.  For the
>>>> +     * EJB Container, the adapter will pass through the method   
>>>> invocations.
>>>> +     */
>>>> +    public String getRemoteClassName() {
>>>> +        return _remoteClassName;
>>>> +    }
>>>> +
>>>> +    public void setRemoteClassName(String rcName) {
>>>> +        _remoteClassName = rcName;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * The remote interface name for the remote object.  This 
>>>> will   most likely be the name
>>>> +     * of the EJB's RemoteInterface and RemoteHomeInterface
>>>> +     *
>>>> +     * The stub/skel generator will use this interface name.
>>>> +     */
>>>> +    public String getRemoteInterfaceName() {
>>>> +        return _remoteInterfaceName;
>>>> +    }
>>>> +
>>>> +    public void setRemoteInterfaceName(String riName) {
>>>> +        _remoteInterfaceName = riName;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * A list of public IDs that the remote object implements:
>>>> +     *
>>>> +     * IDL:....:1.0
>>>> +     * RMI:....:X:Y
>>>> +     */
>>>> +    public Vector getIds() {
>>>> +        return _idVector;
>>>> +    }
>>>> +
>>>> +    public void addId(String id) {
>>>> +        _idVector.add(id);
>>>> +    }
>>>> +
>>>> +    public void removeId(String id) {
>>>> +        _idVector.remove(id);
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * Return the skeleton implemention for the remote 
>>>> interface.    This interface has the
>>>> +     * invoke method to handle the rmi/iiop messages.
>>>> +     */
>>>> +    public RemoteInterface getRemoteInterface() {
>>>> +        if (_ri == null) {
>>>> +            synchronized (this) {
>>>> +                String riName = _remoteInterfaceName + "_Skeleton";
>>>> +                _remoteInterfaceClass = loadClass(riName);
>>>> +
>>>> +                try {
>>>> +                    _ri = (RemoteInterface)   
>>>> _remoteInterfaceClass.newInstance();
>>>> +                } catch (InstantiationException e) {
>>>> +                    e.printStackTrace();  //To change body of 
>>>> catch   statement use File | Settings | File Templates.
>>>> +                } catch (IllegalAccessException e) {
>>>> +                    e.printStackTrace();  //To change body of 
>>>> catch   statement use File | Settings | File Templates.
>>>> +                }
>>>> +            }
>>>> +        }
>>>> +
>>>> +        return _ri;
>>>> +    }
>>>
>>>
>>>
>>> This is double check locking.  The whole method needs to be   
>>> synchronized.
>>
>>
>> MD>> The double check code is wrong and I need to change it.    Does  
>> the method require sync?  Maybe.  I recently came accross a strategy  
>> for multiprocessor locking that uses a transient int as a memory  
>> barrier.  Probably best if I don't attempt to explain it.  We can  
>> optimize our locking.  The template that I came accross is as follows:
>>
>>    private volatile boolean _evaluated = false;
>>
>>    private Object _value;
>>
>>    /**
>>     ** Sub-classes should override this method to get the object's  
>> value
>>     ** when it is first needed.
>>     **/
>>    public abstract Object evaluate();
>>
>>    public final Object getValue()
>>    {
>>        if (_volatileIsEffectiveMemoryBarrier)
>>        {
>>            if (! _evaluated)
>>            {
>>                synchronized (this)
>>                {
>>                    if (! _evaluated)
>>                    {
>>                        _value = evaluate();
>>                        _evaluated = true;
>>                    }
>>                }
>>            }
>>        }
>>        else
>>        {
>>            synchronized (this)
>>            {
>>                if (! _evaluated)
>>                {
>>                    _value = evaluate();
>>                    _evaluated = true;
>>                }
>>            }
>>        }
>>        return _value;
>>    }
>>
>>>
>>>> +    /*
>>>> +     * Get an object instance to invoke based on the object key.
>>>> +     *
>>>> +     * The objectKey could probably be passed to the EJB 
>>>> container  so  that the
>>>> +     * container can directly invoke the ejb object as required.
>>>> +     */
>>>> +    public Object getInstance(byte[] objectKey) {
>>>> +        String key = new String(objectKey);
>>>> +        return getInstance(key);
>>>> +    }
>>>> +
>>>> +    public Object getInstance(String key) {
>>>> +        Object o = _objects.get(key);
>>>> +
>>>> +        if (o == null) {
>>>> +            o = newInstance(key);
>>>> +        }
>>>> +
>>>> +        return o;
>>>> +    }
>>>> +
>>>> +    public Object newInstance(byte[] objectKey) {
>>>> +        String key = new String(objectKey);
>>>> +        return newInstance(key);
>>>> +    }
>>>> +
>>>> +    public Object newInstance(String key) {
>>>> +        Object o = null;
>>>> +
>>>> +        if (_remoteClassClass == null) {
>>>> +            synchronized (this) {
>>>> +                _remoteClassClass = loadClass(_remoteClassName);
>>>> +            }
>>>> +
>>>> +            try {
>>>> +                if (_shared) {
>>>> +                    synchronized (this) {
>>>> +                        Method m =   
>>>> _remoteClassClass.getMethod("getInstance", (Class[]) null);
>>>> +                        o = m.invoke(_remoteClassClass, 
>>>> (Object[])   null);
>>>> +
>>>> +                        if (o != null) {
>>>> +                            _objects.put(key, o);
>>>> +                        }
>>>> +                    }
>>>> +                } else {
>>>> +                    o = _remoteClassClass.newInstance();
>>>> +                    _objects.put(key, o);
>>>> +                }
>>>> +            } catch (InstantiationException e) {
>>>> +                e.printStackTrace();  //To change body of catch   
>>>> statement use File | Settings | File Templates.
>>>> +            } catch (IllegalAccessException e) {
>>>> +                e.printStackTrace();  //To change body of catch   
>>>> statement use File | Settings | File Templates.
>>>> +            } catch (NoSuchMethodException e) {
>>>> +                e.printStackTrace();
>>>> +            } catch (InvocationTargetException e) {
>>>> +                e.printStackTrace();
>>>> +            }
>>>> +        }
>>>> +
>>>> +        return o;
>>>> +    }
>>>
>>>
>>>
>>> Again there is double check locking.  Also, if this is called in 
>>> the   critical path, I suggest we use CGLIB instead of reflection 
>>> for   construction as it is way faster.
>>
>>
>> MD>>  I can look into CGLIB.
>>
>>>
>>>> +    /*
>>>> +     * Invoke method from the IIOP Message Handler.  The adapter 
>>>> is   bound to the INS name service.
>>>> +     * When an RMI/IIOP message is processed by the server, the   
>>>> message handler will perform a lookup
>>>> +     * on the name service to get the Adapter, then the 
>>>> invocation   will be passed to the adapter
>>>> +     * The adapter will obtain the object key and then determine   
>>>> which object instance to pass the
>>>> +     * invocation to.
>>>> +     */
>>>
>>>
>>>
>>> This comment is missing the double star at the top to signify javadoc.
>>>
>>>> +    public void invoke(java.lang.String methodName, byte[]  
>>>> objectKey,  org.apache.geronimo.interop.rmi.iiop.ObjectInputStream  
>>>> input,  org.apache.geronimo.interop.rmi.iiop.ObjectOutputStream  
>>>> output) {
>>>> +        RemoteInterface skeleton = getRemoteInterface();
>>>> +        Object instance = getInstance(objectKey);
>>>> +
>>>> +        if (instance != null) {
>>>> +            skeleton.$invoke(methodName, objectKey, instance,  
>>>> input,  output);
>>>> +        } else {
>>>> +            throw new org.omg.CORBA.OBJECT_NOT_EXIST(new   
>>>> String(objectKey));
>>>> +        }
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * Helper function to load a class.  This uses classloader 
>>>> for   the adapter.
>>>> +     */
>>>> +    protected Class loadClass(String name) {
>>>> +        Class c = null;
>>>> +
>>>> +        try {
>>>> +            c = _cl.loadClass(name);
>>>> +        } catch (Exception e) {
>>>> +            e.printStackTrace();
>>>> +        }
>>>> +
>>>> +        return c;
>>>> +    }
>>>> +}
>>>
>>>
>>>
>>> I think we should be at least logging the error or actually letting  
>>> the  ClassNotFoundException through.  We also have a ClassLoading  
>>> class  which you may want to use as it handles some of the more 
>>> weird  class  name formats.
>>
>>
>> MD >> Will change with better Geronimo integration.
>>
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> adapter/AdapterManager.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/adapter/AdapterManager.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> --- (empty file)
>>>> +++   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> adapter/AdapterManager.java    Sun Jan 23 23:33:10 2005
>>>> @@ -0,0 +1,43 @@
>>>> +/**
>>>> + *
>>>> + *  Copyright 2004-2005 The Apache Software Foundation
>>>> + *
>>>> + *  Licensed under the Apache License, Version 2.0 (the "License");
>>>> + *  you may not use this file except in compliance with the License.
>>>> + *  You may obtain a copy of the License at
>>>> + *
>>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>>> + *
>>>> + *  Unless required by applicable law or agreed to in writing,   
>>>> software
>>>> + *  distributed under the License is distributed on an "AS IS"  
>>>> BASIS,
>>>> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express 
>>>> or   implied.
>>>> + *
>>>> + *  See the License for the specific language governing 
>>>> permissions   and
>>>> + *  limitations under the License.
>>>> + */
>>>> +package org.apache.geronimo.interop.adapter;
>>>> +
>>>> +import java.util.Hashtable;
>>>> +
>>>> +
>>>> +public class AdapterManager {
>>>> +    protected Hashtable _adapters;
>>>> +    protected static AdapterManager _me = new AdapterManager();
>>>
>>>
>>>
>>> Underscores and protected...
>>
>>
>> MD>> Will do.
>>
>>>
>>>> +    protected AdapterManager() {
>>>> +        _adapters = new Hashtable();
>>>> +    }
>>>> +
>>>> +    public static AdapterManager getInstance() {
>>>> +        return _me;
>>>> +    }
>>>
>>>
>>>
>>> This is a bad idea.  Instead we should have a manger gbean and the   
>>> singletonness would be handled by there being only one GBean 
>>> instance   registered.
>>
>>
>> MD>> Sure, this will change with the introduction of GBeans, better  
>> integration...
>>
>>>
>>>> +    public void registerAdapter(Adapter a) {
>>>> +
>>>> +        _adapters.put(a.getBindName(), a);
>>>> +    }
>>>> +
>>>> +    public Adapter getAdapter(String objectName) {
>>>> +        return (Adapter) _adapters.get(objectName);
>>>> +    }
>>>
>>>
>>>
>>> I prefer HashMap and synchronizing the methods that access the Map,  
>>> but  this works in this case.
>>
>>
>> MD>> Hashtables are going to be dead soon.  ;-)
>>
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> client/InitialContextFactory.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ 
>>>> org/apache/geronimo/interop/client/InitialContextFactory.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class InitialContextFactory
>>>> +        implements javax.naming.spi.InitialContextFactory {
>>>> +    private HashMap _startMap = new HashMap();
>>>> +
>>>> +    public Context getInitialContext(java.util.Hashtable env)  
>>>> throws  NamingException {
>>>> +        return   
>>>> org.apache.geronimo.interop.rmi.iiop.client.ClientNamingContext.getIn 
>>>> st ance(env);
>>>> +    }
>>>> +}
>>>
>>>
>>>
>>> Is the _startMap variable used anywhere?
>>
>>
>> MD>> I'd like to remove any naming in the interop and just use the  
>> naming provided in Geronimo.
>>
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> client/InitialContextFactoryBuilder.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  org/apache/geronimo/interop/client/ 
>>>> InitialContextFactoryBuilder.java? view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class InitialContextFactoryBuilder
>>>> +        implements javax.naming.spi.InitialContextFactoryBuilder {
>>>> +    public javax.naming.spi.InitialContextFactory   
>>>> createInitialContextFactory(Hashtable env) {
>>>> +        return new InitialContextFactory();
>>>> +    }
>>>> +}
>>>
>>>
>>>
>>> What is this for?  Just curious...
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> generator/GenException.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/generator/GenException.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +/**
>>>> + * User: Mark
>>>> + * Date: Dec 21, 2004
>>>> + * Time: 3:49:45 PM
>>>> + */
>>>
>>>
>>>
>>> Oops :)
>>>
>>>> +public class GenException
>>>> +        extends Exception {
>>>> +    public GenException() {
>>>> +        super();
>>>> +    }
>>>> +
>>>> +    public GenException(String message) {
>>>> +        super(message);
>>>> +    }
>>>> +
>>>> +    public GenException(Throwable cause) {
>>>> +        super(cause);
>>>> +    }
>>>> +
>>>> +    public GenException(String message, Throwable cause) {
>>>> +        super(message, cause);
>>>> +    }
>>>> +}
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> generator/GenOptions.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/generator/GenOptions.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class GenOptions {
>>>> +    protected String _genDir = "./";
>>>> +    protected boolean _overwrite = false;
>>>> +    protected boolean _verbose = false;
>>>> +
>>>> +    public GenOptions() {
>>>> +    }
>>>> +
>>>> +    public GenOptions(String genDir, boolean overwrite, boolean   
>>>> verbose) {
>>>> +        _genDir = genDir;
>>>> +        _overwrite = overwrite;
>>>> +        _verbose = verbose;
>>>> +    }
>>>> +
>>>> +    public String getGenDir() {
>>>> +        return _genDir;
>>>> +    }
>>>> +
>>>> +    public void setGenDir(String genDir) {
>>>> +        _genDir = genDir;
>>>> +    }
>>>> +
>>>> +    public boolean isOverwrite() {
>>>> +        return _overwrite;
>>>> +    }
>>>> +
>>>> +    public void setOverwrite(boolean overwrite) {
>>>> +        _overwrite = overwrite;
>>>> +    }
>>>> +
>>>> +    public boolean isVerbose() {
>>>> +        return _verbose;
>>>> +    }
>>>> +
>>>> +    public void setVerbose(boolean verbose) {
>>>> +        _verbose = verbose;
>>>> +    }
>>>> +
>>>> +}
>>>
>>>
>>>
>>> Do we need both the constructor args and the setters?  If I set a   
>>> variable is whatever is using this class going to adapt to the  
>>> recently  changed variable?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> naming/NameService.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/naming/NameService.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> --- (empty file)
>>>> +++   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> naming/NameService.java    Sun Jan 23 23:33:10 2005
>>>> @@ -0,0 +1,66 @@
>>>> +/**
>>>> + *
>>>> + *  Copyright 2004-2005 The Apache Software Foundation
>>>> + *
>>>> + *  Licensed under the Apache License, Version 2.0 (the "License");
>>>> + *  you may not use this file except in compliance with the License.
>>>> + *  You may obtain a copy of the License at
>>>> + *
>>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>>> + *
>>>> + *  Unless required by applicable law or agreed to in writing,   
>>>> software
>>>> + *  distributed under the License is distributed on an "AS IS"  
>>>> BASIS,
>>>> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express 
>>>> or   implied.
>>>> + *
>>>> + *  See the License for the specific language governing 
>>>> permissions   and
>>>> + *  limitations under the License.
>>>> + */
>>>> +package org.apache.geronimo.interop.naming;
>>>> +
>>>> +import java.util.HashMap;
>>>> +import javax.naming.NamingException;
>>>> +
>>>> +import org.apache.geronimo.interop.adapter.Adapter;
>>>> +
>>>> +
>>>> +public class NameService {
>>>> +    protected static NameService _ns = null;
>>>
>>>
>>>
>>> protected and underscore
>>>
>>>> +    public static NameService getInstance() {
>>>> +        if (_ns == null) {
>>>> +            synchronized (NameService.class) {
>>>> +                if (_ns == null) {
>>>> +                    _ns = new NameService();
>>>> +                    _ns.init();
>>>> +                }
>>>> +            }
>>>> +        }
>>>> +
>>>> +        return _ns;
>>>> +    }
>>>
>>>
>>>
>>> Double check locking
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> naming/NamingContext.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/naming/NamingContext.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> --- (empty file)
>>>> +++   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> naming/NamingContext.java    Sun Jan 23 23:33:10 2005
>>>> @@ -0,0 +1,150 @@
>>>> +/**
>>>> + *
>>>> + *  Copyright 2004-2005 The Apache Software Foundation
>>>> + *
>>>> + *  Licensed under the Apache License, Version 2.0 (the "License");
>>>> + *  you may not use this file except in compliance with the License.
>>>> + *  You may obtain a copy of the License at
>>>> + *
>>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>>> + *
>>>> + *  Unless required by applicable law or agreed to in writing,   
>>>> software
>>>> + *  distributed under the License is distributed on an "AS IS"  
>>>> BASIS,
>>>> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express 
>>>> or   implied.
>>>> + *
>>>> + *  See the License for the specific language governing 
>>>> permissions   and
>>>> + *  limitations under the License.
>>>> + */
>>>> +package org.apache.geronimo.interop.naming;
>>>> +
>>>> +import java.util.HashMap;
>>>> +import javax.naming.NameNotFoundException;
>>>> +import javax.naming.NamingException;
>>>> +
>>>> +import org.apache.geronimo.interop.adapter.Adapter;
>>>> +
>>>> +
>>>> +public class NamingContext {
>>>> +    public static final NamingContext getInstance(Class baseClass) {
>>>> +        NamingContext context;
>>>> +        synchronized (_contextMap) {
>>>> +            context = (NamingContext) _contextMap.get(baseClass);
>>>> +            if (context == null) {
>>>> +                context = new NamingContext();
>>>> +                _contextMap.put(baseClass, context);
>>>> +                context.init(baseClass);
>>>> +            }
>>>> +        }
>>>> +        return context;
>>>> +    }
>>>> +
>>>> +    private static ThreadLocal _current = new ThreadLocal();
>>>
>>>
>>>
>>> Should this be an InheritableThreadLocal?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/BooleanProperty.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/properties/BooleanProperty.java?  
>>>> view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/ByteProperty.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/properties/ByteProperty.java?  
>>>> view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/DoubleProperty.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/properties/DoubleProperty.java?  
>>>> view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/FloatProperty.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/properties/FloatProperty.java?  
>>>> view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/IntProperty.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/properties/IntProperty.java?  
>>>> view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/LongProperty.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/properties/LongProperty.java?  
>>>> view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/ShortProperty.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/properties/ShortProperty.java?  
>>>> view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/StringProperty.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/properties/StringProperty.java?  
>>>> view=auto&rev=126264
>>>
>>>
>>>
>>> How are these used?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> properties/SystemProperties.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ 
>>>> org/apache/geronimo/interop/properties/SystemProperties.java?  
>>>> view=auto&rev=126264
>>>
>>>
>>>
>>> Is this a bridge to java.lang.System properties?  If so, do we 
>>> really   want to support system properties?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> repository/Repository.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/repository/Repository.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class Repository {
>>>> +    // ??
>>>> +}
>>>
>>>
>>>
>>> Say what? :)
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> rmi/iiop/IiopVersion.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/rmi/iiop/IiopVersion.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class IiopVersion {
>>>> +}
>>>
>>>
>>>
>>> What's this for?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> rmi/iiop/ListenerInfo.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/rmi/iiop/ListenerInfo.java?  
>>>> view=auto&rev=126264
>>>> +public class ListenerInfo {
>>>> +    public int protocol;
>>>> +
>>>> +    public String host;
>>>> +
>>>> +    public int port;
>>>> +}
>>>
>>>
>>>
>>> Do these have to be public fields?  Normally we use, private 
>>> fields   with getters and setters.
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> rmi/iiop/NameServiceOperations_Skeleton.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/rmi/iiop/  
>>>> NameServiceOperations_Skeleton.java?view=auto&rev=126264
>>>
>>>
>>>
>>> Do we want the Skeletons checked in?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> rmi/iiop/ObjectKey.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/rmi/iiop/ObjectKey.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class ObjectKey {
>>>> +    public static final int TYPE_MANAGER = 'M';
>>>> +
>>>> +    public static final int TYPE_SESSION = 'S';
>>>> +
>>>> +    public int type;
>>>> +    public String username = "";
>>>> +    public String password = "";
>>>> +    public String component = "";
>>>> +    public String sessionID = "";
>>>
>>>
>>>
>>> public fields?
>>>
>>>> +    public void checkPassword() {
>>>> +        User.getInstance(username).login(password);
>>>> +    }
>>>
>>>
>>>
>>> Shouldn't this go though Geronimo security?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> rmi/iiop/ObjectRef.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/rmi/iiop/ObjectRef.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +    public int $getIiopVersion() {
>>>> +        return _iiopVersion;
>>>> +    }
>>>
>>>
>>>
>>> Why do all the methods in this class start with a dollar sign?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> rmi/iiop/client/ClientNamingContext.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  org/apache/geronimo/interop/rmi/iiop/client/ 
>>>> ClientNamingContext.java? view=auto&rev=126264
>>>
>>>
>>>
>>> We should consider merging this with the Geronimo naming stuff... 
>>> not  a  task for today, but (much) later on :)
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> rmi/iiop/compiler/StubCompiler.txt
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  
>>>> org/apache/geronimo/interop/rmi/iiop/compiler/StubCompiler.txt?  
>>>> view=auto&rev=126264
>>>
>>>
>>>
>>> Why is this a text file?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> rmi/iiop/server/SocketListener.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  
>>>> org/apache/geronimo/interop/rmi/iiop/server/SocketListener.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class SocketListener extends Thread {
>>>> +    public static SocketListener getInstance() {
>>>> +        return new SocketListener();
>>>> +    }
>>>
>>>
>>>
>>> Why is there a static factory?
>>>
>>>> +    // private data
>>>> +
>>>> +    private String _name;
>>>> +
>>>> +    private String _host;
>>>> +
>>>> +    private int _port;
>>>> +
>>>> +    private int _listenBacklog;
>>>> +
>>>> +    private java.net.ServerSocket _listener;
>>>> +
>>>> +    // internal methods
>>>> +
>>>> +    protected void init() {
>>>> +        _host = "localhost";
>>>> +        _port = 2000;
>>>> +        _listenBacklog = 10;
>>>> +        setDaemon(true);
>>>> +    }
>>>> +
>>>> +    // public methods
>>>> +
>>>> +    public void setHost(String host) {
>>>> +        _host = host;
>>>> +    }
>>>> +
>>>> +    public void setPort(int port) {
>>>> +        _port = port;
>>>> +    }
>>>> +
>>>> +    public void setListenBacklog(int backlog) {
>>>> +        _listenBacklog = backlog;
>>>> +    }
>>>> +
>>>> +    public void run() {
>>>> +        String iiopURL = "iiop://" + _host + ":" + _port;
>>>> +        ListenerInfo listenerInfo = new ListenerInfo();
>>>> +        listenerInfo.protocol = Protocol.IIOP; // TODO: other   
>>>> protocols (IIOPS etc.)
>>>> +        listenerInfo.host = _host;
>>>> +        listenerInfo.port = _port;
>>>> +        try {
>>>> +            InetAddress addr = InetAddress.getByName(_host);
>>>> +            _listener = new java.net.ServerSocket(_port,   
>>>> _listenBacklog, addr);
>>>> +        } catch (Exception ex) {
>>>> +            System.out.println("SocketListener: Error creating  
>>>> server  socket.");
>>>> +            ex.printStackTrace();
>>>> +            try {
>>>> +                Socket socket = new Socket(_host, _port);
>>>> +                socket.close();
>>>> +                System.out.println("SocketListener: Error server   
>>>> already running: " + iiopURL);
>>>> +                ex.printStackTrace();
>>>> +            } catch (Exception ignore) {
>>>> +            }
>>>> +            return;
>>>> +        }
>>>> +        new CheckConnect().start();
>>>> +        for (; ;) {
>>>> +            java.net.Socket socket;
>>>> +            try {
>>>> +                socket = _listener.accept();
>>>> +            } catch (Exception ex) {
>>>> +                throw new SystemException(ex); // TODO: log 
>>>> error   message
>>>> +            }
>>>> +            MessageHandler.getInstance(listenerInfo,  
>>>> socket).start();
>>>> +        }
>>>> +    }
>>>> +
>>>> +    private class CheckConnect extends Thread {
>>>> +        public void run() {
>>>> +            try {
>>>> +                Socket socket = new Socket(_host, _port);
>>>> +                socket.close();
>>>> +                if (!SystemProperties.quiet()) {
>>>> +                      
>>>> System.out.println(formatAcceptingIiopConnections());
>>>> +                }
>>>> +            } catch (Exception ex) {
>>>> +                warnConnectFailed(_host, _port);
>>>> +            }
>>>> +        }
>>>> +    }
>>>> +
>>>> +    // format methods
>>>> +
>>>> +    protected String formatAcceptingIiopConnections() {
>>>> +        String msg =  
>>>> "SocketListener.formatAcceptingIiopConnection():  ";
>>>> +        return msg;
>>>> +    }
>>>> +
>>>> +    // log methods
>>>> +
>>>> +    protected void warnConnectFailed(String host, int port) {
>>>> +        System.out.println("SocketListener.warnConnectFailed():  
>>>> host  = " + host + ", port = " + port);
>>>> +    }
>>>> +}
>>>
>>>
>>>
>>> I suggest, you either require all state data such as host and port 
>>> be   passed in via the constructor, or make the setters throw an 
>>> "already   running" exception.  Otherwise people mistakenly think 
>>> the can  change  the port.
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> security/Role.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  
>>>> org/apache/geronimo/interop/security/Role.java?view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> security/SimpleSubject.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/security/SimpleSubject.java?  
>>>> view=auto&rev=126264
>>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> security/User.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  
>>>> org/apache/geronimo/interop/security/User.java?view=auto&rev=126264
>>>
>>>
>>>
>>> What is the plan for these with regards to the Geronimo security  
>>> module?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> util/SystemUtil.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/  org/apache/geronimo/interop/util/SystemUtil.java? 
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> +public class SystemUtil {
>>>> +    // properties
>>>> +
>>>> +    public static final StringProperty vmVersionProperty =
>>>> +            new StringProperty(SystemProperties.class,   
>>>> "java.vm.version");
>>>> +
>>>> +    // private data
>>>> +
>>>> +    private static String _vmVersion =  
>>>> vmVersionProperty.getString();
>>>> +
>>>> +    private static boolean _isJDK13 = _vmVersion.startsWith("1.3")
>>>> +                                      ||   
>>>> _vmVersion.startsWith("CrE-ME V4.00");
>>>> +
>>>> +    private static boolean _isJDK14 = _vmVersion.startsWith("1.4");
>>>> +
>>>> +    // public methods
>>>> +
>>>> +    public static String getExecutableSuffix() {
>>>> +        return isWindows() ? ".exe" : "";
>>>> +    }
>>>> +
>>>> +    public static String getShellScriptSuffix() {
>>>> +        return isWindows() ? ".bat" : ".sh";
>>>> +    }
>>>> +
>>>> +    public static boolean isJDK13() {
>>>> +        return _isJDK13;
>>>> +    }
>>>> +
>>>> +    public static boolean isJDK14() {
>>>> +        return _isJDK14;
>>>> +    }
>>>> +
>>>> +    public static boolean isWindows() {
>>>> +        return java.io.File.separatorChar == '\\';
>>>> +    }
>>>> +}
>>>
>>>
>>>
>>> How is this used?
>>>
>>>> Added:   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> util/ThreadContext.java
>>>> Url:   
>>>> http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/ 
>>>> java/ org/apache/geronimo/interop/util/ThreadContext.java?  
>>>> view=auto&rev=126264
>>>> ===================================================================== 
>>>> == =======
>>>> --- (empty file)
>>>> +++   
>>>> geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/  
>>>> util/ThreadContext.java    Sun Jan 23 23:33:10 2005
>>>> @@ -0,0 +1,135 @@
>>>> +/**
>>>> + *
>>>> + *  Copyright 2004-2005 The Apache Software Foundation
>>>> + *
>>>> + *  Licensed under the Apache License, Version 2.0 (the "License");
>>>> + *  you may not use this file except in compliance with the License.
>>>> + *  You may obtain a copy of the License at
>>>> + *
>>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>>> + *
>>>> + *  Unless required by applicable law or agreed to in writing,   
>>>> software
>>>> + *  distributed under the License is distributed on an "AS IS"  
>>>> BASIS,
>>>> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express 
>>>> or   implied.
>>>> + *
>>>> + *  See the License for the specific language governing 
>>>> permissions   and
>>>> + *  limitations under the License.
>>>> + */
>>>> +package org.apache.geronimo.interop.util;
>>>> +
>>>> +import java.util.HashMap;
>>>> +
>>>> +import org.apache.geronimo.interop.SystemException;
>>>> +
>>>> +
>>>> +public abstract class ThreadContext {
>>>> +    private static HashMap _primTypes;
>>>> +
>>>> +    private static ThreadLocal _defaultRmiHost = new ThreadLocal();
>>>> +
>>>> +    private static ThreadLocal _defaultRmiPort = new ThreadLocal();
>>>> +
>>>> +    static {
>>>> +        _primTypes = new HashMap();
>>>> +        _primTypes.put("boolean", boolean.class);
>>>> +        _primTypes.put("char", char.class);
>>>> +        _primTypes.put("byte", byte.class);
>>>> +        _primTypes.put("short", short.class);
>>>> +        _primTypes.put("int", int.class);
>>>> +        _primTypes.put("long", long.class);
>>>> +        _primTypes.put("float", float.class);
>>>> +        _primTypes.put("double", double.class);
>>>> +        _primTypes.put("boolean[]", boolean[].class);
>>>> +        _primTypes.put("char[]", char[].class);
>>>> +        _primTypes.put("byte[]", byte[].class);
>>>> +        _primTypes.put("short[]", short[].class);
>>>> +        _primTypes.put("int[]", int[].class);
>>>> +        _primTypes.put("long[]", long[].class);
>>>> +        _primTypes.put("float[]", float[].class);
>>>> +        _primTypes.put("double[]", double[].class);
>>>> +    }
>>>> +
>>>> +    public static String getDefaultRmiHost() {
>>>> +        String host = (String) _defaultRmiHost.get();
>>>> +        if (host == null) {
>>>> +            host = "0";
>>>> +        }
>>>> +        return host;
>>>> +    }
>>>> +
>>>> +    public static int getDefaultRmiPort() {
>>>> +        Integer port = (Integer) _defaultRmiPort.get();
>>>> +        if (port == null) {
>>>> +            port = IntegerCache.get(0);
>>>> +        }
>>>> +        return port.intValue();
>>>> +    }
>>>> +
>>>> +    public static Class loadClass(String className) {
>>>> +        Class t = (Class) _primTypes.get(className);
>>>> +        if (t != null) {
>>>> +            return t;
>>>> +        }
>>>> +        try {
>>>> +            ClassLoader loader =   
>>>> Thread.currentThread().getContextClassLoader();
>>>> +            if (loader == null) {
>>>> +                return Class.forName(className);
>>>> +            } else {
>>>> +                return loader.loadClass(className);
>>>> +            }
>>>> +        } catch (RuntimeException ex) {
>>>> +            throw (RuntimeException) ex;
>>>> +        } catch (Exception ex) {
>>>> +            throw new SystemException(ex);
>>>> +        }
>>>> +    }
>>>
>>>
>>>
>>> This is unlikely to work in Geronimo as we almost never set 
>>> context   class loader.  Normally we try to explicitly pass around 
>>> the   classloader.
>>>
>>>> +
>>>> +    public static Class loadClass(String className, Class   
>>>> parentClass) {
>>>> +        if (parentClass == null) {
>>>> +            return loadClass(className);
>>>> +        }
>>>> +        Class t = (Class) _primTypes.get(className);
>>>> +        if (t != null) {
>>>> +            return t;
>>>> +        }
>>>> +        try {
>>>> +            ClassLoader loader = parentClass.getClassLoader();
>>>> +            if (loader == null) {
>>>> +                return loadClass(className);
>>>> +            } else {
>>>> +                return loader.loadClass(className);
>>>> +            }
>>>> +        } catch (RuntimeException ex) {
>>>> +            throw (RuntimeException) ex;
>>>> +        } catch (Exception ex) {
>>>> +            throw new SystemException(ex);
>>>> +        }
>>>> +    }
>>>> +
>>>> +    public static Class loadClassOrReturnNullIfNotFound(String   
>>>> className) {
>>>> +        try {
>>>> +            return loadClass(className);
>>>> +        } catch (RuntimeException ex) {
>>>> +            return null;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    public static Class loadClassOrReturnNullIfNotFound(String   
>>>> className, Class parentClass) {
>>>> +        if (parentClass == null) {
>>>> +            return loadClassOrReturnNullIfNotFound(className);
>>>> +        }
>>>> +        try {
>>>> +            return loadClass(className, parentClass);
>>>> +        } catch (RuntimeException ex) {
>>>> +            return null;
>>>> +        }
>>>> +    }
>>>
>>>
>>>
>>> Most of this class loader stuff is handled by the class 
>>> ClassLoading   which handles some of the more weird stuff.
>>>
>>>
>>> .
>>>
>>
>
>
> .
>