You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Jarek Gawor <jg...@gmail.com> on 2010/01/20 20:38:14 UTC

rfc66 fun

Hey,

For last few days I've been looking into getting the rfc66 extender
going in Geronimo but I ran into a problem. As previously mentioned on
this list, the idea for the extender was to call the Tomcat/Jetty
ModuleBuilders with a bundle as an input and once the deployment
process was done start the generated configuration. All without
creating any additional temporary or permanent bundles.

Because we don't have a single classloader that can load all the
gbeans within the configuration, David Jencks added a special
"classSource" attribute to GBeanData which can be used to figure out
the right Bundle to load the gbean class. Now, since we use Java
serialization to save and load gbeans, we actually need to set the
right classloader when we deserialize the gbean. That is, during
deserialization as soon as we read the "classSource" we need to lookup
and set the right classloader and then read the rest of the gbean
data. This should (and seems to) work when all the attributes of the
gbean are accessible from the same classloader. But what about if the
gbean has some attribute with some values from different classloaders?
For example, the Map object we build for jndi context can contain
objects from different classloaders. I'm not exactly sure what to do
about it. Although maybe having a custom ObjectOutputStream with
annotateClass() method that saves "classSource" type of info for each
unique class might work. Ideas?

Also, I'm dealing with lots of classloader issues since there is no
single classloader that load all the gbean classes and module classes.
A lot of Geronimo and other code assumes a single classloader and
resolving these problems is time consuming and not very fun (although
probably good in long term). So I'm wondering if we can still somehow
assemble a single classloader in the extender. For example, the
http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
post shows a classloader that delegates a number of different bundles.
Maybe we could use that in Geronimo especially since we can figure out
the bundles needed from the configuration environment information.

Thoughts?

Jarek

Re: rfc66 fun

Posted by Rex Wang <rw...@gmail.com>.
2010/1/21 David Jencks <da...@yahoo.com>

>
> On Jan 20, 2010, at 8:14 PM, Jarek Gawor wrote:
>
>  David,
>>
>> This patch assumes that the entire gbean data can be loaded either via
>> classSource bundle or configuration bundle. But that's not necessarily
>> true. For example, web module builder setups a gbean for the web app
>> context. That gbean has classSource set to tomcat or jetty
>> configuration. One of the attributes of that gbean is a Map object
>> that represents the jndi context for the application. During
>> deployment different naming builders put objects into that map. These
>> naming builders have totally separate classloaders (from web module
>> builder) so the objects within the Map will be of different
>> classloaders. And that means that during deserialization we can't read
>> in the entire Map object using just the jetty or tomcat configuration.
>>
>
> I see what you mean.  In 2.2 we handle this by having the naming builders
> add to the environment so the eventual plugin can load the classes from
> these additions.
>
>
>
>> My proposed patch is attached. My solution does serialize bundle
>> symbolic names and use PackageAdmin to resolve them but it would be
>> very easy to switch it to another method of resolving the symbolic
>> names.
>>
>> Also, maybe this entire classSource problem goes away if we can use a
>> ClassLoader that delegates to a bunch of Bundles.
>>
>
> I think I remember an idea of having, as well as the bundle for the
> application, another bundle that import everything in the application, and
> everything needed for the gbeans for the application.  On a similar line,
> maybe we could construct an additional bundle that imports all the packages
> that are used as we serialize the gbean data.  This bundle would be the
> "classSource" bundle able to load everything to allow deserializing the
> gbean data, but it would allow normal osgi mechanisms to work to figure out
> where the classes are actually coming from.  I'd guess that instead of
> writing the bundle symbolic name with each class, we could collect the
> packages and then use them for a manifest import-Package clause.
>
I agree with this approach, since we choose osgi framework to help the
classloading in G3.0, we should not manually do it by ourselves again.
And do you mean a fragment bundle or an individual car bundle for each app
bundle?

-Rex


> Do you think this might be worth pursuing?  I'm fine with just applying
> your proposal while we think about this more.
>
> thanks
> david jencks
>
>
>> Jarek
>>
>> On Wed, Jan 20, 2010 at 5:44 PM, David Jencks <da...@yahoo.com>
>> wrote:
>>
>>>
>>> On Jan 20, 2010, at 11:38 AM, Jarek Gawor wrote:
>>>
>>>  Hey,
>>>>
>>>> For last few days I've been looking into getting the rfc66 extender
>>>> going in Geronimo but I ran into a problem. As previously mentioned on
>>>> this list, the idea for the extender was to call the Tomcat/Jetty
>>>> ModuleBuilders with a bundle as an input and once the deployment
>>>> process was done start the generated configuration. All without
>>>> creating any additional temporary or permanent bundles.
>>>>
>>>> Because we don't have a single classloader that can load all the
>>>> gbeans within the configuration, David Jencks added a special
>>>> "classSource" attribute to GBeanData which can be used to figure out
>>>> the right Bundle to load the gbean class. Now, since we use Java
>>>> serialization to save and load gbeans, we actually need to set the
>>>> right classloader when we deserialize the gbean. That is, during
>>>> deserialization as soon as we read the "classSource" we need to lookup
>>>> and set the right classloader and then read the rest of the gbean
>>>> data. This should (and seems to) work when all the attributes of the
>>>> gbean are accessible from the same classloader. But what about if the
>>>> gbean has some attribute with some values from different classloaders?
>>>> For example, the Map object we build for jndi context can contain
>>>> objects from different classloaders. I'm not exactly sure what to do
>>>> about it. Although maybe having a custom ObjectOutputStream with
>>>> annotateClass() method that saves "classSource" type of info for each
>>>> unique class might work. Ideas?
>>>>
>>>> Also, I'm dealing with lots of classloader issues since there is no
>>>> single classloader that load all the gbean classes and module classes.
>>>> A lot of Geronimo and other code assumes a single classloader and
>>>> resolving these problems is time consuming and not very fun (although
>>>> probably good in long term). So I'm wondering if we can still somehow
>>>> assemble a single classloader in the extender. For example, the
>>>> http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
>>>> post shows a classloader that delegates a number of different bundles.
>>>> Maybe we could use that in Geronimo especially since we can figure out
>>>> the bundles needed from the configuration environment information.
>>>>
>>>> Thoughts?
>>>>
>>>> Jarek
>>>>
>>>
>>> I think that we are either going to load the gbean from the classSource
>>> bundle or from the configuration's bundle.  Maybe sometimes we need to
>>> mix
>>> them... not sure.  Anyway I think we can modify the ObjectInputStreamExt
>>> to
>>> look in the classSource's bundle or in the bundle as in the attached
>>> patch.
>>>  I chatted with Jarek a bit on irc and I think he is thinking of writing
>>> the
>>> bundle symbolic name into the output stream and using package admin to
>>> look
>>> it up again.  I'm a little worried with this approach that we may be
>>> mixing
>>> too much osgi into geronimo.  If we had a blueprint namespace handler I
>>> think we'd be able to load the classes in the namespace handler rather
>>> than
>>> needing to rely on something that is really like requireBundle.  So, my
>>> approach is somewhat similar but uses geronimo techniques rather than
>>> osgi
>>> techniques.  I think this might be appropriate since we are in fact
>>> loading
>>> gbeans.
>>>
>>> my patch compiles but I haven't tried it yet.
>>>
>>> thoughts?
>>>
>>> david jencks
>>>
>>> Index:
>>>
>>> src/test/java/org/apache/geronimo/kernel/mock/MockConfigurationManager.java
>>> ===================================================================
>>> ---
>>>
>>> src/test/java/org/apache/geronimo/kernel/mock/MockConfigurationManager.java
>>> (revision 901109)
>>> +++
>>>
>>> src/test/java/org/apache/geronimo/kernel/mock/MockConfigurationManager.java
>>> (working copy)
>>> @@ -122,7 +122,7 @@
>>>    public LifecycleResults loadConfiguration(ConfigurationData
>>> configurationData, LifecycleMonitor monitor) throws
>>> NoSuchConfigException,
>>> LifecycleException {
>>>        try {
>>>            Artifact configId =
>>> configurationData.getEnvironment().getConfigId();
>>> -            Configuration configuration = new
>>> Configuration(configurationData, new DependencyNode(configId, null,
>>> null),
>>> null, null, null);
>>> +            Configuration configuration = new
>>> Configuration(configurationData, new DependencyNode(configId, null,
>>> null),
>>> null, null, null, null);
>>>            configurations.put(configId, configuration);
>>>        } catch (InvalidConfigException e) {
>>>
>>> Index:
>>>
>>> src/test/java/org/apache/geronimo/kernel/config/ConfigurationUtilTest.java
>>> ===================================================================
>>> ---
>>>
>>> src/test/java/org/apache/geronimo/kernel/config/ConfigurationUtilTest.java
>>>  (revision 901109)
>>> +++
>>>
>>> src/test/java/org/apache/geronimo/kernel/config/ConfigurationUtilTest.java
>>>  (working copy)
>>> @@ -74,10 +74,10 @@
>>>
>>>    private void assertEquals(ConfigurationData data, ConfigurationData
>>> configurationData) throws InvalidConfigException {
>>>        List gbeans;
>>> -        gbeans = data.getGBeans(bundleContext.getBundle());
>>> +        gbeans = data.getGBeans(bundleContext.getBundle(), null);
>>>        assertEquals(configurationData.getId(), data.getId());
>>>        ConfigurationData data3 = (ConfigurationData)
>>> data.getChildConfigurations().get("testmodule");
>>> -        gbeans = data3.getGBeans(bundleContext.getBundle());
>>> +        gbeans = data3.getGBeans(bundleContext.getBundle(), null);
>>>        assertEquals(new QName("namespaceURI", "localPart"),
>>> ((GBeanData)gbeans.get(0)).getAttribute("someObject"));
>>>    }
>>>
>>> Index: src/main/java/org/apache/geronimo/kernel/ObjectInputStreamExt.java
>>> ===================================================================
>>> --- src/main/java/org/apache/geronimo/kernel/ObjectInputStreamExt.java
>>>  (revision 901109)
>>> +++ src/main/java/org/apache/geronimo/kernel/ObjectInputStreamExt.java
>>>  (working copy)
>>> @@ -22,6 +22,9 @@
>>>  import java.io.ObjectStreamClass;
>>>  import java.lang.reflect.Proxy;
>>>
>>> +import org.apache.geronimo.kernel.config.Configuration;
>>> +import org.apache.geronimo.kernel.config.InvalidConfigException;
>>> +import org.apache.geronimo.kernel.repository.Artifact;
>>>  import org.osgi.framework.Bundle;
>>>
>>>  /**
>>> @@ -29,19 +32,41 @@
>>>  */
>>>  public class ObjectInputStreamExt extends ObjectInputStream {
>>>
>>> -    private Bundle bundle;
>>> +    private final Bundle bundle;
>>> +    private final Kernel kernel;
>>> +    private Bundle sourceBundle;
>>>
>>> -    public ObjectInputStreamExt(InputStream in, Bundle bundle) throws
>>> IOException {
>>> +    public ObjectInputStreamExt(InputStream in, Bundle bundle, Kernel
>>> kernel) throws IOException {
>>>        super(in);
>>>        this.bundle = bundle;
>>> +        this.kernel = kernel;
>>>    }
>>>
>>> +    public void setClassSource(Artifact classSource) throws
>>> ClassNotFoundException {
>>> +        if (classSource != null) {
>>> +            try {
>>> +                Configuration configuration = (Configuration)
>>> kernel.getGBean(Configuration.getConfigurationAbstractName(classSource));
>>> +                sourceBundle = configuration.getBundle();
>>> +            } catch (GBeanNotFoundException e) {
>>> +                throw new ClassNotFoundException("Could not locate
>>> Configuration to deserialize", e);
>>> +            } catch (InvalidConfigException e) {
>>> +                throw new ClassNotFoundException("Could not identify
>>> Configuration to deserialize", e);
>>> +            }
>>> +        } else {
>>> +            sourceBundle = null;
>>> +        }
>>> +    }
>>> +
>>>    protected Class resolveClass(ObjectStreamClass classDesc) throws
>>> IOException, ClassNotFoundException {
>>> +        if (sourceBundle != null) {
>>> +            return ClassLoading.loadClass(classDesc.getName(), bundle);
>>> +        }
>>>        return ClassLoading.loadClass(classDesc.getName(), bundle);
>>>    }
>>>
>>>  //see
>>> http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
>>>    //currently broken
>>> +
>>>    protected Class resolveProxyClass(String[] interfaces) throws
>>> IOException, ClassNotFoundException {
>>>        Class[] cinterfaces = new Class[interfaces.length];
>>>        for (int i = 0; i < interfaces.length; i++)
>>> Index:
>>> src/main/java/org/apache/geronimo/kernel/config/ConfigurationData.java
>>> ===================================================================
>>> ---
>>> src/main/java/org/apache/geronimo/kernel/config/ConfigurationData.java
>>>   (revision 901109)
>>> +++
>>> src/main/java/org/apache/geronimo/kernel/config/ConfigurationData.java
>>>   (working copy)
>>> @@ -28,6 +28,7 @@
>>>
>>>  import org.apache.geronimo.gbean.GBeanData;
>>>  import org.apache.geronimo.gbean.GBeanInfo;
>>> +import org.apache.geronimo.kernel.Kernel;
>>>  import org.apache.geronimo.kernel.Naming;
>>>  import org.apache.geronimo.kernel.repository.Artifact;
>>>  import org.apache.geronimo.kernel.repository.Environment;
>>> @@ -179,9 +180,9 @@
>>>        return environment.getManifest();
>>>    }
>>>
>>> -    public List<GBeanData> getGBeans(Bundle bundle) throws
>>> InvalidConfigException {
>>> +    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)
>>> throws
>>> InvalidConfigException {
>>>        if (bundle == null) throw new NullPointerException("bundle is
>>> null");
>>> -        List<GBeanData> gbeans = gbeanState.getGBeans(bundle);
>>> +        List<GBeanData> gbeans = gbeanState.getGBeans(bundle, kernel);
>>>        if (null == configurationDataTransformer) {
>>>            return gbeans;
>>>        }
>>> Index:
>>>
>>> src/main/java/org/apache/geronimo/kernel/config/xstream/XStreamGBeanState.java
>>> ===================================================================
>>> ---
>>>
>>> src/main/java/org/apache/geronimo/kernel/config/xstream/XStreamGBeanState.java
>>>     (revision 901109)
>>> +++
>>>
>>> src/main/java/org/apache/geronimo/kernel/config/xstream/XStreamGBeanState.java
>>>     (working copy)
>>> @@ -32,6 +32,7 @@
>>>  import org.apache.geronimo.gbean.AbstractName;
>>>  import org.apache.geronimo.gbean.GBeanData;
>>>  import org.apache.geronimo.gbean.GBeanInfo;
>>> +import org.apache.geronimo.kernel.Kernel;
>>>  import org.apache.geronimo.kernel.Naming;
>>>  import org.apache.geronimo.kernel.util.XmlUtil;
>>>  import org.apache.geronimo.kernel.config.InvalidConfigException;
>>> @@ -73,7 +74,7 @@
>>>        return gbeanState;
>>>    }
>>>
>>> -    public List<GBeanData> getGBeans(Bundle bundle) throws
>>> InvalidConfigException {
>>> +    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)
>>> throws
>>> InvalidConfigException {
>>>        if (gbeanState == null) {
>>>            return Collections.unmodifiableList(gbeans);
>>>        }
>>> Index: src/main/java/org/apache/geronimo/kernel/config/Configuration.java
>>> ===================================================================
>>> --- src/main/java/org/apache/geronimo/kernel/config/Configuration.java
>>>  (revision 901109)
>>> +++ src/main/java/org/apache/geronimo/kernel/config/Configuration.java
>>>  (working copy)
>>> @@ -38,8 +38,11 @@
>>>  import org.apache.geronimo.gbean.ReferencePatterns;
>>>  import org.apache.geronimo.gbean.annotation.GBean;
>>>  import org.apache.geronimo.gbean.annotation.ParamAttribute;
>>> +import org.apache.geronimo.gbean.annotation.ParamSpecial;
>>> +import org.apache.geronimo.gbean.annotation.SpecialAttributeType;
>>>  import org.apache.geronimo.kernel.GBeanAlreadyExistsException;
>>>  import org.apache.geronimo.kernel.GBeanNotFoundException;
>>> +import org.apache.geronimo.kernel.Kernel;
>>>  import org.apache.geronimo.kernel.Naming;
>>>  import org.apache.geronimo.kernel.repository.Artifact;
>>>  import org.apache.geronimo.kernel.repository.Environment;
>>> @@ -86,6 +89,7 @@
>>>     * Converts an Artifact to an AbstractName for a configuration.  Does
>>> not
>>>     * validate that this is a reasonable or resolved Artifact, or that it
>>>     * corresponds to an actual Configuration.
>>> +     *
>>>     * @param configId id for configuration
>>>     * @return abstract name constructed from supplied id
>>>     * @throws InvalidConfigException if the ObjectName could not be
>>> constructed
>>> @@ -167,23 +171,24 @@
>>>
>>>    /**
>>>     * Creates a configuration.
>>> +     * <p/>
>>> +     * //     * @param classLoaderHolder Classloaders for this
>>> configuration
>>>     *
>>> -//     * @param classLoaderHolder Classloaders for this configuration
>>> -     * @param configurationData the module type, environment and
>>> classpath
>>> of the configuration
>>> -     * @param dependencyNode Class and Service parent ids
>>> -     * @param allServiceParents ordered list of transitive closure of
>>> service parents for gbean searches
>>> -     * @param attributeStore Customization info for gbeans
>>> +     * @param configurationData     the module type, environment and
>>> classpath of the configuration
>>> +     * @param dependencyNode        Class and Service parent ids
>>> +     * @param allServiceParents     ordered list of transitive closure
>>> of
>>> service parents for gbean searches
>>> +     * @param attributeStore        Customization info for gbeans
>>>     * @param configurationResolver (there should be a better way) Where
>>> this configuration is actually located in file system
>>> +     * @param kernel
>>>     * @throws InvalidConfigException if this configuration turns out to
>>> have a problem.
>>>     */
>>>    public Configuration(
>>> -//            @ParamAttribute(name = "classLoaderHolder")
>>> ClassLoaderHolder
>>> classLoaderHolder,
>>>            @ParamAttribute(name = "configurationData") ConfigurationData
>>> configurationData,
>>>            @ParamAttribute(name = "dependencyNode") DependencyNode
>>> dependencyNode,
>>>            @ParamAttribute(name = "allServiceParents")
>>> List<Configuration>
>>> allServiceParents,
>>>            @ParamAttribute(name = "attributeStore")
>>> ManageableAttributeStore attributeStore,
>>> -            @ParamAttribute(name = "configurationResolver")
>>> ConfigurationResolver configurationResolver) throws
>>> InvalidConfigException {
>>> -//        if (classLoaderHolder == null) throw new
>>> NullPointerException("classLoaders are null");
>>> +            @ParamAttribute(name = "configurationResolver")
>>> ConfigurationResolver configurationResolver,
>>> +            @ParamSpecial(type = SpecialAttributeType.kernel) Kernel
>>> kernel) throws InvalidConfigException {
>>>        if (configurationData == null) throw new
>>> NullPointerException("configurationData is null");
>>>
>>>  //        this.classLoaderHolder = classLoaderHolder;
>>> @@ -199,7 +204,7 @@
>>>
>>>            // Deserialize the GBeans in the configurationData
>>>            Bundle bundle =
>>> configurationData.getBundleContext().getBundle();
>>> -            Collection<GBeanData> gbeans =
>>> configurationData.getGBeans(bundle);
>>> +            Collection<GBeanData> gbeans =
>>> configurationData.getGBeans(bundle, kernel);
>>>            if (attributeStore != null) {
>>>                gbeans =
>>> attributeStore.applyOverrides(dependencyNode.getId(), gbeans, bundle);
>>>            }
>>> @@ -221,6 +226,7 @@
>>>
>>>    /**
>>>     * Add a contained configuration, such as for a war inside an ear
>>> +     *
>>>     * @param child contained configuration
>>>     */
>>>    void addChild(Configuration child) {
>>> @@ -311,6 +317,7 @@
>>>
>>>    /**
>>>     * Provide a way to locate where this configuration is for web apps
>>> and
>>> persistence units
>>> +     *
>>>     * @return the ConfigurationResolver for this configuration
>>>     */
>>>    public ConfigurationResolver getConfigurationResolver() {
>>> Index: src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
>>> ===================================================================
>>> --- src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
>>> (revision 901109)
>>> +++ src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
>>> (working copy)
>>> @@ -20,6 +20,7 @@
>>>
>>>  import org.apache.geronimo.gbean.GBeanData;
>>>  import org.apache.geronimo.gbean.GBeanInfo;
>>> +import org.apache.geronimo.kernel.Kernel;
>>>  import org.apache.geronimo.kernel.Naming;
>>>  import org.apache.geronimo.kernel.repository.Environment;
>>>  import org.osgi.framework.Bundle;
>>> @@ -28,7 +29,7 @@
>>>  * @version $Rev$ $Date$
>>>  */
>>>  public interface GBeanState {
>>> -    List<GBeanData> getGBeans(Bundle bundle) throws
>>> InvalidConfigException;
>>> +    List<GBeanData> getGBeans(Bundle bundle, Kernel kernel) throws
>>> InvalidConfigException;
>>>
>>>    void addGBean(GBeanData gbeanData);
>>>
>>> Index:
>>> src/main/java/org/apache/geronimo/kernel/config/SerializedGBeanState.java
>>> ===================================================================
>>> ---
>>> src/main/java/org/apache/geronimo/kernel/config/SerializedGBeanState.java
>>> (revision 901109)
>>> +++
>>> src/main/java/org/apache/geronimo/kernel/config/SerializedGBeanState.java
>>> (working copy)
>>> @@ -31,6 +31,7 @@
>>>  import org.apache.geronimo.gbean.AbstractName;
>>>  import org.apache.geronimo.gbean.GBeanData;
>>>  import org.apache.geronimo.gbean.GBeanInfo;
>>> +import org.apache.geronimo.kernel.Kernel;
>>>  import org.apache.geronimo.kernel.Naming;
>>>  import org.apache.geronimo.kernel.ObjectInputStreamExt;
>>>  import org.apache.geronimo.kernel.repository.Environment;
>>> @@ -58,11 +59,11 @@
>>>        }
>>>    }
>>>
>>> -    public List<GBeanData> getGBeans(Bundle bundle) throws
>>> InvalidConfigException {
>>> +    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)
>>> throws
>>> InvalidConfigException {
>>>        if (gbeanState == null) {
>>>            return Collections.unmodifiableList(gbeans);
>>>        }
>>> -        gbeans.addAll(loadGBeans(gbeanState, bundle));
>>> +        gbeans.addAll(loadGBeans(gbeanState, bundle, kernel));
>>>        return Collections.unmodifiableList(gbeans);
>>>    }
>>>
>>> @@ -110,7 +111,7 @@
>>>        stream.defaultWriteObject();
>>>    }
>>>
>>> -    private static List<GBeanData> loadGBeans(byte[] gbeanState, Bundle
>>> bundle) throws InvalidConfigException {
>>> +    private static List<GBeanData> loadGBeans(byte[] gbeanState, Bundle
>>> bundle, Kernel kernel) throws InvalidConfigException {
>>>        List<GBeanData> gbeans = new ArrayList<GBeanData>();
>>>        if (gbeanState != null && gbeanState.length > 0) {
>>>            // Set the thread context classloader so deserializing classes
>>> can grab the cl from the thread
>>> @@ -118,7 +119,7 @@
>>>            try {
>>>  //
>>>  Thread.currentThread().setContextClassLoader(classLoader);
>>>
>>> -                ObjectInputStream ois = new ObjectInputStreamExt(new
>>> ByteArrayInputStream(gbeanState), bundle);
>>> +                ObjectInputStream ois = new ObjectInputStreamExt(new
>>> ByteArrayInputStream(gbeanState), bundle, kernel);
>>>                try {
>>>                    while (true) {
>>>                        GBeanData gbeanData = new GBeanData();
>>> Index:
>>>
>>> src/main/java/org/apache/geronimo/kernel/config/SimpleConfigurationManager.java
>>> ===================================================================
>>> ---
>>>
>>> src/main/java/org/apache/geronimo/kernel/config/SimpleConfigurationManager.java
>>>    (revision 901109)
>>> +++
>>>
>>> src/main/java/org/apache/geronimo/kernel/config/SimpleConfigurationManager.java
>>>    (working copy)
>>> @@ -424,7 +424,7 @@
>>>
>>>        List<Configuration> allServiceParents =
>>> buildAllServiceParents(null,
>>> dependencyNode);
>>>
>>> -        Configuration configuration = new
>>> Configuration(configurationData,
>>> dependencyNode, allServiceParents, null, configurationResolver);
>>> +        Configuration configuration = new
>>> Configuration(configurationData,
>>> dependencyNode, allServiceParents, null, configurationResolver, null);
>>>        configuration.doStart();
>>>        //TODO why???
>>>        resolvedParentIds.add(configuration.getId());
>>> @@ -1478,7 +1478,8 @@
>>>     */
>>>    private void applyOverrides(Configuration configuration) throws
>>> InvalidConfigException {
>>>        Bundle bundle = configuration.getBundle();
>>> -        Collection<GBeanData> gbeans =
>>> configuration.getConfigurationData().getGBeans(bundle);
>>> +        //TODO OSGI we might need a kernel here?
>>> +        Collection<GBeanData> gbeans =
>>> configuration.getConfigurationData().getGBeans(bundle, null);
>>>        if (configuration.getManageableAttributeStore() != null) {
>>>
>>>
>>> configuration.getManageableAttributeStore().applyOverrides(configuration.getId(),
>>> gbeans,
>>>                    bundle);
>>> Index: src/main/java/org/apache/geronimo/gbean/GBeanData.java
>>> ===================================================================
>>> --- src/main/java/org/apache/geronimo/gbean/GBeanData.java      (revision
>>> 901109)
>>> +++ src/main/java/org/apache/geronimo/gbean/GBeanData.java      (working
>>> copy)
>>> @@ -28,6 +28,7 @@
>>>  import java.util.Set;
>>>
>>>  import org.apache.geronimo.gbean.annotation.EncryptionSetting;
>>> +import org.apache.geronimo.kernel.ObjectInputStreamExt;
>>>  import org.apache.geronimo.kernel.repository.Artifact;
>>>
>>>  /**
>>> @@ -419,6 +420,9 @@
>>>
>>>        protected void readClassSource(ObjectInput in) throws
>>> ClassNotFoundException, IOException {
>>>            classSource = (Artifact) in.readObject();
>>> +            if (in instanceof ObjectInputStreamExt) {
>>> +                ((ObjectInputStreamExt)in).setClassSource(classSource);
>>> +            }
>>>        }
>>>    }
>>>
>>>
>>>
>>>  <gbean.patch>
>>
>
>


-- 
Lei Wang (Rex)
rwonly AT apache.org

Re: rfc66 fun

Posted by David Jencks <da...@yahoo.com>.
On Jan 20, 2010, at 8:14 PM, Jarek Gawor wrote:

> David,
>
> This patch assumes that the entire gbean data can be loaded either via
> classSource bundle or configuration bundle. But that's not necessarily
> true. For example, web module builder setups a gbean for the web app
> context. That gbean has classSource set to tomcat or jetty
> configuration. One of the attributes of that gbean is a Map object
> that represents the jndi context for the application. During
> deployment different naming builders put objects into that map. These
> naming builders have totally separate classloaders (from web module
> builder) so the objects within the Map will be of different
> classloaders. And that means that during deserialization we can't read
> in the entire Map object using just the jetty or tomcat configuration.

I see what you mean.  In 2.2 we handle this by having the naming  
builders add to the environment so the eventual plugin can load the  
classes from these additions.

>
> My proposed patch is attached. My solution does serialize bundle
> symbolic names and use PackageAdmin to resolve them but it would be
> very easy to switch it to another method of resolving the symbolic
> names.
>
> Also, maybe this entire classSource problem goes away if we can use a
> ClassLoader that delegates to a bunch of Bundles.

I think I remember an idea of having, as well as the bundle for the  
application, another bundle that import everything in the application,  
and everything needed for the gbeans for the application.  On a  
similar line, maybe we could construct an additional bundle that  
imports all the packages that are used as we serialize the gbean  
data.  This bundle would be the "classSource" bundle able to load  
everything to allow deserializing the gbean data, but it would allow  
normal osgi mechanisms to work to figure out where the classes are  
actually coming from.  I'd guess that instead of writing the bundle  
symbolic name with each class, we could collect the packages and then  
use them for a manifest import-Package clause.

Do you think this might be worth pursuing?  I'm fine with just  
applying your proposal while we think about this more.

thanks
david jencks

>
> Jarek
>
> On Wed, Jan 20, 2010 at 5:44 PM, David Jencks  
> <da...@yahoo.com> wrote:
>>
>> On Jan 20, 2010, at 11:38 AM, Jarek Gawor wrote:
>>
>>> Hey,
>>>
>>> For last few days I've been looking into getting the rfc66 extender
>>> going in Geronimo but I ran into a problem. As previously  
>>> mentioned on
>>> this list, the idea for the extender was to call the Tomcat/Jetty
>>> ModuleBuilders with a bundle as an input and once the deployment
>>> process was done start the generated configuration. All without
>>> creating any additional temporary or permanent bundles.
>>>
>>> Because we don't have a single classloader that can load all the
>>> gbeans within the configuration, David Jencks added a special
>>> "classSource" attribute to GBeanData which can be used to figure out
>>> the right Bundle to load the gbean class. Now, since we use Java
>>> serialization to save and load gbeans, we actually need to set the
>>> right classloader when we deserialize the gbean. That is, during
>>> deserialization as soon as we read the "classSource" we need to  
>>> lookup
>>> and set the right classloader and then read the rest of the gbean
>>> data. This should (and seems to) work when all the attributes of the
>>> gbean are accessible from the same classloader. But what about if  
>>> the
>>> gbean has some attribute with some values from different  
>>> classloaders?
>>> For example, the Map object we build for jndi context can contain
>>> objects from different classloaders. I'm not exactly sure what to do
>>> about it. Although maybe having a custom ObjectOutputStream with
>>> annotateClass() method that saves "classSource" type of info for  
>>> each
>>> unique class might work. Ideas?
>>>
>>> Also, I'm dealing with lots of classloader issues since there is no
>>> single classloader that load all the gbean classes and module  
>>> classes.
>>> A lot of Geronimo and other code assumes a single classloader and
>>> resolving these problems is time consuming and not very fun  
>>> (although
>>> probably good in long term). So I'm wondering if we can still  
>>> somehow
>>> assemble a single classloader in the extender. For example, the
>>> http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
>>> post shows a classloader that delegates a number of different  
>>> bundles.
>>> Maybe we could use that in Geronimo especially since we can figure  
>>> out
>>> the bundles needed from the configuration environment information.
>>>
>>> Thoughts?
>>>
>>> Jarek
>>
>> I think that we are either going to load the gbean from the  
>> classSource
>> bundle or from the configuration's bundle.  Maybe sometimes we need  
>> to mix
>> them... not sure.  Anyway I think we can modify the  
>> ObjectInputStreamExt to
>> look in the classSource's bundle or in the bundle as in the  
>> attached patch.
>>  I chatted with Jarek a bit on irc and I think he is thinking of  
>> writing the
>> bundle symbolic name into the output stream and using package admin  
>> to look
>> it up again.  I'm a little worried with this approach that we may  
>> be mixing
>> too much osgi into geronimo.  If we had a blueprint namespace  
>> handler I
>> think we'd be able to load the classes in the namespace handler  
>> rather than
>> needing to rely on something that is really like requireBundle.   
>> So, my
>> approach is somewhat similar but uses geronimo techniques rather  
>> than osgi
>> techniques.  I think this might be appropriate since we are in fact  
>> loading
>> gbeans.
>>
>> my patch compiles but I haven't tried it yet.
>>
>> thoughts?
>>
>> david jencks
>>
>> Index:
>> src/test/java/org/apache/geronimo/kernel/mock/ 
>> MockConfigurationManager.java
>> ===================================================================
>> ---
>> src/test/java/org/apache/geronimo/kernel/mock/ 
>> MockConfigurationManager.java
>> (revision 901109)
>> +++
>> src/test/java/org/apache/geronimo/kernel/mock/ 
>> MockConfigurationManager.java
>> (working copy)
>> @@ -122,7 +122,7 @@
>>     public LifecycleResults loadConfiguration(ConfigurationData
>> configurationData, LifecycleMonitor monitor) throws  
>> NoSuchConfigException,
>> LifecycleException {
>>         try {
>>             Artifact configId =
>> configurationData.getEnvironment().getConfigId();
>> -            Configuration configuration = new
>> Configuration(configurationData, new DependencyNode(configId, null,  
>> null),
>> null, null, null);
>> +            Configuration configuration = new
>> Configuration(configurationData, new DependencyNode(configId, null,  
>> null),
>> null, null, null, null);
>>             configurations.put(configId, configuration);
>>         } catch (InvalidConfigException e) {
>>
>> Index:
>> src/test/java/org/apache/geronimo/kernel/config/ 
>> ConfigurationUtilTest.java
>> ===================================================================
>> ---
>> src/test/java/org/apache/geronimo/kernel/config/ 
>> ConfigurationUtilTest.java
>>  (revision 901109)
>> +++
>> src/test/java/org/apache/geronimo/kernel/config/ 
>> ConfigurationUtilTest.java
>>  (working copy)
>> @@ -74,10 +74,10 @@
>>
>>     private void assertEquals(ConfigurationData data,  
>> ConfigurationData
>> configurationData) throws InvalidConfigException {
>>         List gbeans;
>> -        gbeans = data.getGBeans(bundleContext.getBundle());
>> +        gbeans = data.getGBeans(bundleContext.getBundle(), null);
>>         assertEquals(configurationData.getId(), data.getId());
>>         ConfigurationData data3 = (ConfigurationData)
>> data.getChildConfigurations().get("testmodule");
>> -        gbeans = data3.getGBeans(bundleContext.getBundle());
>> +        gbeans = data3.getGBeans(bundleContext.getBundle(), null);
>>         assertEquals(new QName("namespaceURI", "localPart"),
>> ((GBeanData)gbeans.get(0)).getAttribute("someObject"));
>>     }
>>
>> Index: src/main/java/org/apache/geronimo/kernel/ 
>> ObjectInputStreamExt.java
>> ===================================================================
>> --- src/main/java/org/apache/geronimo/kernel/ 
>> ObjectInputStreamExt.java
>>  (revision 901109)
>> +++ src/main/java/org/apache/geronimo/kernel/ 
>> ObjectInputStreamExt.java
>>  (working copy)
>> @@ -22,6 +22,9 @@
>>  import java.io.ObjectStreamClass;
>>  import java.lang.reflect.Proxy;
>>
>> +import org.apache.geronimo.kernel.config.Configuration;
>> +import org.apache.geronimo.kernel.config.InvalidConfigException;
>> +import org.apache.geronimo.kernel.repository.Artifact;
>>  import org.osgi.framework.Bundle;
>>
>>  /**
>> @@ -29,19 +32,41 @@
>>  */
>>  public class ObjectInputStreamExt extends ObjectInputStream {
>>
>> -    private Bundle bundle;
>> +    private final Bundle bundle;
>> +    private final Kernel kernel;
>> +    private Bundle sourceBundle;
>>
>> -    public ObjectInputStreamExt(InputStream in, Bundle bundle)  
>> throws
>> IOException {
>> +    public ObjectInputStreamExt(InputStream in, Bundle bundle,  
>> Kernel
>> kernel) throws IOException {
>>         super(in);
>>         this.bundle = bundle;
>> +        this.kernel = kernel;
>>     }
>>
>> +    public void setClassSource(Artifact classSource) throws
>> ClassNotFoundException {
>> +        if (classSource != null) {
>> +            try {
>> +                Configuration configuration = (Configuration)
>> kernel 
>> .getGBean(Configuration.getConfigurationAbstractName(classSource));
>> +                sourceBundle = configuration.getBundle();
>> +            } catch (GBeanNotFoundException e) {
>> +                throw new ClassNotFoundException("Could not locate
>> Configuration to deserialize", e);
>> +            } catch (InvalidConfigException e) {
>> +                throw new ClassNotFoundException("Could not identify
>> Configuration to deserialize", e);
>> +            }
>> +        } else {
>> +            sourceBundle = null;
>> +        }
>> +    }
>> +
>>     protected Class resolveClass(ObjectStreamClass classDesc) throws
>> IOException, ClassNotFoundException {
>> +        if (sourceBundle != null) {
>> +            return ClassLoading.loadClass(classDesc.getName(),  
>> bundle);
>> +        }
>>         return ClassLoading.loadClass(classDesc.getName(), bundle);
>>     }
>>
>>  //see
>> http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
>>     //currently broken
>> +
>>     protected Class resolveProxyClass(String[] interfaces) throws
>> IOException, ClassNotFoundException {
>>         Class[] cinterfaces = new Class[interfaces.length];
>>         for (int i = 0; i < interfaces.length; i++)
>> Index:
>> src/main/java/org/apache/geronimo/kernel/config/ 
>> ConfigurationData.java
>> ===================================================================
>> --- src/main/java/org/apache/geronimo/kernel/config/ 
>> ConfigurationData.java
>>    (revision 901109)
>> +++ src/main/java/org/apache/geronimo/kernel/config/ 
>> ConfigurationData.java
>>    (working copy)
>> @@ -28,6 +28,7 @@
>>
>>  import org.apache.geronimo.gbean.GBeanData;
>>  import org.apache.geronimo.gbean.GBeanInfo;
>> +import org.apache.geronimo.kernel.Kernel;
>>  import org.apache.geronimo.kernel.Naming;
>>  import org.apache.geronimo.kernel.repository.Artifact;
>>  import org.apache.geronimo.kernel.repository.Environment;
>> @@ -179,9 +180,9 @@
>>         return environment.getManifest();
>>     }
>>
>> -    public List<GBeanData> getGBeans(Bundle bundle) throws
>> InvalidConfigException {
>> +    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)  
>> throws
>> InvalidConfigException {
>>         if (bundle == null) throw new NullPointerException("bundle is
>> null");
>> -        List<GBeanData> gbeans = gbeanState.getGBeans(bundle);
>> +        List<GBeanData> gbeans = gbeanState.getGBeans(bundle,  
>> kernel);
>>         if (null == configurationDataTransformer) {
>>             return gbeans;
>>         }
>> Index:
>> src/main/java/org/apache/geronimo/kernel/config/xstream/ 
>> XStreamGBeanState.java
>> ===================================================================
>> ---
>> src/main/java/org/apache/geronimo/kernel/config/xstream/ 
>> XStreamGBeanState.java
>>      (revision 901109)
>> +++
>> src/main/java/org/apache/geronimo/kernel/config/xstream/ 
>> XStreamGBeanState.java
>>      (working copy)
>> @@ -32,6 +32,7 @@
>>  import org.apache.geronimo.gbean.AbstractName;
>>  import org.apache.geronimo.gbean.GBeanData;
>>  import org.apache.geronimo.gbean.GBeanInfo;
>> +import org.apache.geronimo.kernel.Kernel;
>>  import org.apache.geronimo.kernel.Naming;
>>  import org.apache.geronimo.kernel.util.XmlUtil;
>>  import org.apache.geronimo.kernel.config.InvalidConfigException;
>> @@ -73,7 +74,7 @@
>>         return gbeanState;
>>     }
>>
>> -    public List<GBeanData> getGBeans(Bundle bundle) throws
>> InvalidConfigException {
>> +    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)  
>> throws
>> InvalidConfigException {
>>         if (gbeanState == null) {
>>             return Collections.unmodifiableList(gbeans);
>>         }
>> Index: src/main/java/org/apache/geronimo/kernel/config/ 
>> Configuration.java
>> ===================================================================
>> --- src/main/java/org/apache/geronimo/kernel/config/ 
>> Configuration.java
>>  (revision 901109)
>> +++ src/main/java/org/apache/geronimo/kernel/config/ 
>> Configuration.java
>>  (working copy)
>> @@ -38,8 +38,11 @@
>>  import org.apache.geronimo.gbean.ReferencePatterns;
>>  import org.apache.geronimo.gbean.annotation.GBean;
>>  import org.apache.geronimo.gbean.annotation.ParamAttribute;
>> +import org.apache.geronimo.gbean.annotation.ParamSpecial;
>> +import org.apache.geronimo.gbean.annotation.SpecialAttributeType;
>>  import org.apache.geronimo.kernel.GBeanAlreadyExistsException;
>>  import org.apache.geronimo.kernel.GBeanNotFoundException;
>> +import org.apache.geronimo.kernel.Kernel;
>>  import org.apache.geronimo.kernel.Naming;
>>  import org.apache.geronimo.kernel.repository.Artifact;
>>  import org.apache.geronimo.kernel.repository.Environment;
>> @@ -86,6 +89,7 @@
>>      * Converts an Artifact to an AbstractName for a  
>> configuration.  Does
>> not
>>      * validate that this is a reasonable or resolved Artifact, or  
>> that it
>>      * corresponds to an actual Configuration.
>> +     *
>>      * @param configId id for configuration
>>      * @return abstract name constructed from supplied id
>>      * @throws InvalidConfigException if the ObjectName could not be
>> constructed
>> @@ -167,23 +171,24 @@
>>
>>     /**
>>      * Creates a configuration.
>> +     * <p/>
>> +     * //     * @param classLoaderHolder Classloaders for this
>> configuration
>>      *
>> -//     * @param classLoaderHolder Classloaders for this  
>> configuration
>> -     * @param configurationData the module type, environment and  
>> classpath
>> of the configuration
>> -     * @param dependencyNode Class and Service parent ids
>> -     * @param allServiceParents ordered list of transitive closure  
>> of
>> service parents for gbean searches
>> -     * @param attributeStore Customization info for gbeans
>> +     * @param configurationData     the module type, environment and
>> classpath of the configuration
>> +     * @param dependencyNode        Class and Service parent ids
>> +     * @param allServiceParents     ordered list of transitive  
>> closure of
>> service parents for gbean searches
>> +     * @param attributeStore        Customization info for gbeans
>>      * @param configurationResolver (there should be a better way)  
>> Where
>> this configuration is actually located in file system
>> +     * @param kernel
>>      * @throws InvalidConfigException if this configuration turns  
>> out to
>> have a problem.
>>      */
>>     public Configuration(
>> -//            @ParamAttribute(name = "classLoaderHolder")  
>> ClassLoaderHolder
>> classLoaderHolder,
>>             @ParamAttribute(name = "configurationData")  
>> ConfigurationData
>> configurationData,
>>             @ParamAttribute(name = "dependencyNode") DependencyNode
>> dependencyNode,
>>             @ParamAttribute(name = "allServiceParents")  
>> List<Configuration>
>> allServiceParents,
>>             @ParamAttribute(name = "attributeStore")
>> ManageableAttributeStore attributeStore,
>> -            @ParamAttribute(name = "configurationResolver")
>> ConfigurationResolver configurationResolver) throws  
>> InvalidConfigException {
>> -//        if (classLoaderHolder == null) throw new
>> NullPointerException("classLoaders are null");
>> +            @ParamAttribute(name = "configurationResolver")
>> ConfigurationResolver configurationResolver,
>> +            @ParamSpecial(type = SpecialAttributeType.kernel) Kernel
>> kernel) throws InvalidConfigException {
>>         if (configurationData == null) throw new
>> NullPointerException("configurationData is null");
>>
>>  //        this.classLoaderHolder = classLoaderHolder;
>> @@ -199,7 +204,7 @@
>>
>>             // Deserialize the GBeans in the configurationData
>>             Bundle bundle =
>> configurationData.getBundleContext().getBundle();
>> -            Collection<GBeanData> gbeans =
>> configurationData.getGBeans(bundle);
>> +            Collection<GBeanData> gbeans =
>> configurationData.getGBeans(bundle, kernel);
>>             if (attributeStore != null) {
>>                 gbeans =
>> attributeStore.applyOverrides(dependencyNode.getId(), gbeans,  
>> bundle);
>>             }
>> @@ -221,6 +226,7 @@
>>
>>     /**
>>      * Add a contained configuration, such as for a war inside an ear
>> +     *
>>      * @param child contained configuration
>>      */
>>     void addChild(Configuration child) {
>> @@ -311,6 +317,7 @@
>>
>>     /**
>>      * Provide a way to locate where this configuration is for web  
>> apps and
>> persistence units
>> +     *
>>      * @return the ConfigurationResolver for this configuration
>>      */
>>     public ConfigurationResolver getConfigurationResolver() {
>> Index: src/main/java/org/apache/geronimo/kernel/config/ 
>> GBeanState.java
>> ===================================================================
>> --- src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
>> (revision 901109)
>> +++ src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
>> (working copy)
>> @@ -20,6 +20,7 @@
>>
>>  import org.apache.geronimo.gbean.GBeanData;
>>  import org.apache.geronimo.gbean.GBeanInfo;
>> +import org.apache.geronimo.kernel.Kernel;
>>  import org.apache.geronimo.kernel.Naming;
>>  import org.apache.geronimo.kernel.repository.Environment;
>>  import org.osgi.framework.Bundle;
>> @@ -28,7 +29,7 @@
>>  * @version $Rev$ $Date$
>>  */
>>  public interface GBeanState {
>> -    List<GBeanData> getGBeans(Bundle bundle) throws  
>> InvalidConfigException;
>> +    List<GBeanData> getGBeans(Bundle bundle, Kernel kernel) throws
>> InvalidConfigException;
>>
>>     void addGBean(GBeanData gbeanData);
>>
>> Index:
>> src/main/java/org/apache/geronimo/kernel/config/ 
>> SerializedGBeanState.java
>> ===================================================================
>> ---
>> src/main/java/org/apache/geronimo/kernel/config/ 
>> SerializedGBeanState.java
>> (revision 901109)
>> +++
>> src/main/java/org/apache/geronimo/kernel/config/ 
>> SerializedGBeanState.java
>> (working copy)
>> @@ -31,6 +31,7 @@
>>  import org.apache.geronimo.gbean.AbstractName;
>>  import org.apache.geronimo.gbean.GBeanData;
>>  import org.apache.geronimo.gbean.GBeanInfo;
>> +import org.apache.geronimo.kernel.Kernel;
>>  import org.apache.geronimo.kernel.Naming;
>>  import org.apache.geronimo.kernel.ObjectInputStreamExt;
>>  import org.apache.geronimo.kernel.repository.Environment;
>> @@ -58,11 +59,11 @@
>>         }
>>     }
>>
>> -    public List<GBeanData> getGBeans(Bundle bundle) throws
>> InvalidConfigException {
>> +    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)  
>> throws
>> InvalidConfigException {
>>         if (gbeanState == null) {
>>             return Collections.unmodifiableList(gbeans);
>>         }
>> -        gbeans.addAll(loadGBeans(gbeanState, bundle));
>> +        gbeans.addAll(loadGBeans(gbeanState, bundle, kernel));
>>         return Collections.unmodifiableList(gbeans);
>>     }
>>
>> @@ -110,7 +111,7 @@
>>         stream.defaultWriteObject();
>>     }
>>
>> -    private static List<GBeanData> loadGBeans(byte[] gbeanState,  
>> Bundle
>> bundle) throws InvalidConfigException {
>> +    private static List<GBeanData> loadGBeans(byte[] gbeanState,  
>> Bundle
>> bundle, Kernel kernel) throws InvalidConfigException {
>>         List<GBeanData> gbeans = new ArrayList<GBeanData>();
>>         if (gbeanState != null && gbeanState.length > 0) {
>>             // Set the thread context classloader so deserializing  
>> classes
>> can grab the cl from the thread
>> @@ -118,7 +119,7 @@
>>             try {
>>  //
>>  Thread.currentThread().setContextClassLoader(classLoader);
>>
>> -                ObjectInputStream ois = new ObjectInputStreamExt(new
>> ByteArrayInputStream(gbeanState), bundle);
>> +                ObjectInputStream ois = new ObjectInputStreamExt(new
>> ByteArrayInputStream(gbeanState), bundle, kernel);
>>                 try {
>>                     while (true) {
>>                         GBeanData gbeanData = new GBeanData();
>> Index:
>> src/main/java/org/apache/geronimo/kernel/config/ 
>> SimpleConfigurationManager.java
>> ===================================================================
>> ---
>> src/main/java/org/apache/geronimo/kernel/config/ 
>> SimpleConfigurationManager.java
>>     (revision 901109)
>> +++
>> src/main/java/org/apache/geronimo/kernel/config/ 
>> SimpleConfigurationManager.java
>>     (working copy)
>> @@ -424,7 +424,7 @@
>>
>>         List<Configuration> allServiceParents =  
>> buildAllServiceParents(null,
>> dependencyNode);
>>
>> -        Configuration configuration = new  
>> Configuration(configurationData,
>> dependencyNode, allServiceParents, null, configurationResolver);
>> +        Configuration configuration = new  
>> Configuration(configurationData,
>> dependencyNode, allServiceParents, null, configurationResolver,  
>> null);
>>         configuration.doStart();
>>         //TODO why???
>>         resolvedParentIds.add(configuration.getId());
>> @@ -1478,7 +1478,8 @@
>>      */
>>     private void applyOverrides(Configuration configuration) throws
>> InvalidConfigException {
>>         Bundle bundle = configuration.getBundle();
>> -        Collection<GBeanData> gbeans =
>> configuration.getConfigurationData().getGBeans(bundle);
>> +        //TODO OSGI we might need a kernel here?
>> +        Collection<GBeanData> gbeans =
>> configuration.getConfigurationData().getGBeans(bundle, null);
>>         if (configuration.getManageableAttributeStore() != null) {
>>
>> configuration 
>> .getManageableAttributeStore().applyOverrides(configuration.getId(),
>> gbeans,
>>                     bundle);
>> Index: src/main/java/org/apache/geronimo/gbean/GBeanData.java
>> ===================================================================
>> --- src/main/java/org/apache/geronimo/gbean/GBeanData.java       
>> (revision
>> 901109)
>> +++ src/main/java/org/apache/geronimo/gbean/GBeanData.java       
>> (working
>> copy)
>> @@ -28,6 +28,7 @@
>>  import java.util.Set;
>>
>>  import org.apache.geronimo.gbean.annotation.EncryptionSetting;
>> +import org.apache.geronimo.kernel.ObjectInputStreamExt;
>>  import org.apache.geronimo.kernel.repository.Artifact;
>>
>>  /**
>> @@ -419,6 +420,9 @@
>>
>>         protected void readClassSource(ObjectInput in) throws
>> ClassNotFoundException, IOException {
>>             classSource = (Artifact) in.readObject();
>> +            if (in instanceof ObjectInputStreamExt) {
>> +                 
>> ((ObjectInputStreamExt)in).setClassSource(classSource);
>> +            }
>>         }
>>     }
>>
>>
>>
> <gbean.patch>


Re: rfc66 fun

Posted by Jarek Gawor <jg...@gmail.com>.
David,

This patch assumes that the entire gbean data can be loaded either via
classSource bundle or configuration bundle. But that's not necessarily
true. For example, web module builder setups a gbean for the web app
context. That gbean has classSource set to tomcat or jetty
configuration. One of the attributes of that gbean is a Map object
that represents the jndi context for the application. During
deployment different naming builders put objects into that map. These
naming builders have totally separate classloaders (from web module
builder) so the objects within the Map will be of different
classloaders. And that means that during deserialization we can't read
in the entire Map object using just the jetty or tomcat configuration.

My proposed patch is attached. My solution does serialize bundle
symbolic names and use PackageAdmin to resolve them but it would be
very easy to switch it to another method of resolving the symbolic
names.

Also, maybe this entire classSource problem goes away if we can use a
ClassLoader that delegates to a bunch of Bundles.

Jarek

On Wed, Jan 20, 2010 at 5:44 PM, David Jencks <da...@yahoo.com> wrote:
>
> On Jan 20, 2010, at 11:38 AM, Jarek Gawor wrote:
>
>> Hey,
>>
>> For last few days I've been looking into getting the rfc66 extender
>> going in Geronimo but I ran into a problem. As previously mentioned on
>> this list, the idea for the extender was to call the Tomcat/Jetty
>> ModuleBuilders with a bundle as an input and once the deployment
>> process was done start the generated configuration. All without
>> creating any additional temporary or permanent bundles.
>>
>> Because we don't have a single classloader that can load all the
>> gbeans within the configuration, David Jencks added a special
>> "classSource" attribute to GBeanData which can be used to figure out
>> the right Bundle to load the gbean class. Now, since we use Java
>> serialization to save and load gbeans, we actually need to set the
>> right classloader when we deserialize the gbean. That is, during
>> deserialization as soon as we read the "classSource" we need to lookup
>> and set the right classloader and then read the rest of the gbean
>> data. This should (and seems to) work when all the attributes of the
>> gbean are accessible from the same classloader. But what about if the
>> gbean has some attribute with some values from different classloaders?
>> For example, the Map object we build for jndi context can contain
>> objects from different classloaders. I'm not exactly sure what to do
>> about it. Although maybe having a custom ObjectOutputStream with
>> annotateClass() method that saves "classSource" type of info for each
>> unique class might work. Ideas?
>>
>> Also, I'm dealing with lots of classloader issues since there is no
>> single classloader that load all the gbean classes and module classes.
>> A lot of Geronimo and other code assumes a single classloader and
>> resolving these problems is time consuming and not very fun (although
>> probably good in long term). So I'm wondering if we can still somehow
>> assemble a single classloader in the extender. For example, the
>> http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
>> post shows a classloader that delegates a number of different bundles.
>> Maybe we could use that in Geronimo especially since we can figure out
>> the bundles needed from the configuration environment information.
>>
>> Thoughts?
>>
>> Jarek
>
> I think that we are either going to load the gbean from the classSource
> bundle or from the configuration's bundle. �Maybe sometimes we need to mix
> them... not sure. �Anyway I think we can modify the ObjectInputStreamExt to
> look in the classSource's bundle or in the bundle as in the attached patch.
> �I chatted with Jarek a bit on irc and I think he is thinking of writing the
> bundle symbolic name into the output stream and using package admin to look
> it up again. �I'm a little worried with this approach that we may be mixing
> too much osgi into geronimo. �If we had a blueprint namespace handler I
> think we'd be able to load the classes in the namespace handler rather than
> needing to rely on something that is really like requireBundle. �So, my
> approach is somewhat similar but uses geronimo techniques rather than osgi
> techniques. �I think this might be appropriate since we are in fact loading
> gbeans.
>
> my patch compiles but I haven't tried it yet.
>
> thoughts?
>
> david jencks
>
> Index:
> src/test/java/org/apache/geronimo/kernel/mock/MockConfigurationManager.java
> ===================================================================
> ---
> src/test/java/org/apache/geronimo/kernel/mock/MockConfigurationManager.java
> (revision 901109)
> +++
> src/test/java/org/apache/geronimo/kernel/mock/MockConfigurationManager.java
> (working copy)
> @@ -122,7 +122,7 @@
> � � public LifecycleResults loadConfiguration(ConfigurationData
> configurationData, LifecycleMonitor monitor) throws NoSuchConfigException,
> LifecycleException {
> � � � � try {
> � � � � � � Artifact configId =
> configurationData.getEnvironment().getConfigId();
> - � � � � � �Configuration configuration = new
> Configuration(configurationData, new DependencyNode(configId, null, null),
> null, null, null);
> + � � � � � �Configuration configuration = new
> Configuration(configurationData, new DependencyNode(configId, null, null),
> null, null, null, null);
> � � � � � � configurations.put(configId, configuration);
> � � � � } catch (InvalidConfigException e) {
>
> Index:
> src/test/java/org/apache/geronimo/kernel/config/ConfigurationUtilTest.java
> ===================================================================
> ---
> src/test/java/org/apache/geronimo/kernel/config/ConfigurationUtilTest.java
> �(revision 901109)
> +++
> src/test/java/org/apache/geronimo/kernel/config/ConfigurationUtilTest.java
> �(working copy)
> @@ -74,10 +74,10 @@
>
> � � private void assertEquals(ConfigurationData data, ConfigurationData
> configurationData) throws InvalidConfigException {
> � � � � List gbeans;
> - � � � �gbeans = data.getGBeans(bundleContext.getBundle());
> + � � � �gbeans = data.getGBeans(bundleContext.getBundle(), null);
> � � � � assertEquals(configurationData.getId(), data.getId());
> � � � � ConfigurationData data3 = (ConfigurationData)
> data.getChildConfigurations().get("testmodule");
> - � � � �gbeans = data3.getGBeans(bundleContext.getBundle());
> + � � � �gbeans = data3.getGBeans(bundleContext.getBundle(), null);
> � � � � assertEquals(new QName("namespaceURI", "localPart"),
> ((GBeanData)gbeans.get(0)).getAttribute("someObject"));
> � � }
>
> Index: src/main/java/org/apache/geronimo/kernel/ObjectInputStreamExt.java
> ===================================================================
> --- src/main/java/org/apache/geronimo/kernel/ObjectInputStreamExt.java
> �(revision 901109)
> +++ src/main/java/org/apache/geronimo/kernel/ObjectInputStreamExt.java
> �(working copy)
> @@ -22,6 +22,9 @@
> �import java.io.ObjectStreamClass;
> �import java.lang.reflect.Proxy;
>
> +import org.apache.geronimo.kernel.config.Configuration;
> +import org.apache.geronimo.kernel.config.InvalidConfigException;
> +import org.apache.geronimo.kernel.repository.Artifact;
> �import org.osgi.framework.Bundle;
>
> �/**
> @@ -29,19 +32,41 @@
> �*/
> �public class ObjectInputStreamExt extends ObjectInputStream {
>
> - � �private Bundle bundle;
> + � �private final Bundle bundle;
> + � �private final Kernel kernel;
> + � �private Bundle sourceBundle;
>
> - � �public ObjectInputStreamExt(InputStream in, Bundle bundle) throws
> IOException {
> + � �public ObjectInputStreamExt(InputStream in, Bundle bundle, Kernel
> kernel) throws IOException {
> � � � � super(in);
> � � � � this.bundle = bundle;
> + � � � �this.kernel = kernel;
> � � }
>
> + � �public void setClassSource(Artifact classSource) throws
> ClassNotFoundException {
> + � � � �if (classSource != null) {
> + � � � � � �try {
> + � � � � � � � �Configuration configuration = (Configuration)
> kernel.getGBean(Configuration.getConfigurationAbstractName(classSource));
> + � � � � � � � �sourceBundle = configuration.getBundle();
> + � � � � � �} catch (GBeanNotFoundException e) {
> + � � � � � � � �throw new ClassNotFoundException("Could not locate
> Configuration to deserialize", e);
> + � � � � � �} catch (InvalidConfigException e) {
> + � � � � � � � �throw new ClassNotFoundException("Could not identify
> Configuration to deserialize", e);
> + � � � � � �}
> + � � � �} else {
> + � � � � � �sourceBundle = null;
> + � � � �}
> + � �}
> +
> � � protected Class resolveClass(ObjectStreamClass classDesc) throws
> IOException, ClassNotFoundException {
> + � � � �if (sourceBundle != null) {
> + � � � � � �return ClassLoading.loadClass(classDesc.getName(), bundle);
> + � � � �}
> � � � � return ClassLoading.loadClass(classDesc.getName(), bundle);
> � � }
>
> �//see
> http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
> � � //currently broken
> +
> � � protected Class resolveProxyClass(String[] interfaces) throws
> IOException, ClassNotFoundException {
> � � � � Class[] cinterfaces = new Class[interfaces.length];
> � � � � for (int i = 0; i < interfaces.length; i++)
> Index:
> src/main/java/org/apache/geronimo/kernel/config/ConfigurationData.java
> ===================================================================
> --- src/main/java/org/apache/geronimo/kernel/config/ConfigurationData.java
> � �(revision 901109)
> +++ src/main/java/org/apache/geronimo/kernel/config/ConfigurationData.java
> � �(working copy)
> @@ -28,6 +28,7 @@
>
> �import org.apache.geronimo.gbean.GBeanData;
> �import org.apache.geronimo.gbean.GBeanInfo;
> +import org.apache.geronimo.kernel.Kernel;
> �import org.apache.geronimo.kernel.Naming;
> �import org.apache.geronimo.kernel.repository.Artifact;
> �import org.apache.geronimo.kernel.repository.Environment;
> @@ -179,9 +180,9 @@
> � � � � return environment.getManifest();
> � � }
>
> - � �public List<GBeanData> getGBeans(Bundle bundle) throws
> InvalidConfigException {
> + � �public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel) throws
> InvalidConfigException {
> � � � � if (bundle == null) throw new NullPointerException("bundle is
> null");
> - � � � �List<GBeanData> gbeans = gbeanState.getGBeans(bundle);
> + � � � �List<GBeanData> gbeans = gbeanState.getGBeans(bundle, kernel);
> � � � � if (null == configurationDataTransformer) {
> � � � � � � return gbeans;
> � � � � }
> Index:
> src/main/java/org/apache/geronimo/kernel/config/xstream/XStreamGBeanState.java
> ===================================================================
> ---
> src/main/java/org/apache/geronimo/kernel/config/xstream/XStreamGBeanState.java
> � � �(revision 901109)
> +++
> src/main/java/org/apache/geronimo/kernel/config/xstream/XStreamGBeanState.java
> � � �(working copy)
> @@ -32,6 +32,7 @@
> �import org.apache.geronimo.gbean.AbstractName;
> �import org.apache.geronimo.gbean.GBeanData;
> �import org.apache.geronimo.gbean.GBeanInfo;
> +import org.apache.geronimo.kernel.Kernel;
> �import org.apache.geronimo.kernel.Naming;
> �import org.apache.geronimo.kernel.util.XmlUtil;
> �import org.apache.geronimo.kernel.config.InvalidConfigException;
> @@ -73,7 +74,7 @@
> � � � � return gbeanState;
> � � }
>
> - � �public List<GBeanData> getGBeans(Bundle bundle) throws
> InvalidConfigException {
> + � �public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel) throws
> InvalidConfigException {
> � � � � if (gbeanState == null) {
> � � � � � � return Collections.unmodifiableList(gbeans);
> � � � � }
> Index: src/main/java/org/apache/geronimo/kernel/config/Configuration.java
> ===================================================================
> --- src/main/java/org/apache/geronimo/kernel/config/Configuration.java
> �(revision 901109)
> +++ src/main/java/org/apache/geronimo/kernel/config/Configuration.java
> �(working copy)
> @@ -38,8 +38,11 @@
> �import org.apache.geronimo.gbean.ReferencePatterns;
> �import org.apache.geronimo.gbean.annotation.GBean;
> �import org.apache.geronimo.gbean.annotation.ParamAttribute;
> +import org.apache.geronimo.gbean.annotation.ParamSpecial;
> +import org.apache.geronimo.gbean.annotation.SpecialAttributeType;
> �import org.apache.geronimo.kernel.GBeanAlreadyExistsException;
> �import org.apache.geronimo.kernel.GBeanNotFoundException;
> +import org.apache.geronimo.kernel.Kernel;
> �import org.apache.geronimo.kernel.Naming;
> �import org.apache.geronimo.kernel.repository.Artifact;
> �import org.apache.geronimo.kernel.repository.Environment;
> @@ -86,6 +89,7 @@
> � � �* Converts an Artifact to an AbstractName for a configuration. �Does
> not
> � � �* validate that this is a reasonable or resolved Artifact, or that it
> � � �* corresponds to an actual Configuration.
> + � � *
> � � �* @param configId id for configuration
> � � �* @return abstract name constructed from supplied id
> � � �* @throws InvalidConfigException if the ObjectName could not be
> constructed
> @@ -167,23 +171,24 @@
>
> � � /**
> � � �* Creates a configuration.
> + � � * <p/>
> + � � * // � � * @param classLoaderHolder Classloaders for this
> configuration
> � � �*
> -// � � * @param classLoaderHolder Classloaders for this configuration
> - � � * @param configurationData the module type, environment and classpath
> of the configuration
> - � � * @param dependencyNode Class and Service parent ids
> - � � * @param allServiceParents ordered list of transitive closure of
> service parents for gbean searches
> - � � * @param attributeStore Customization info for gbeans
> + � � * @param configurationData � � the module type, environment and
> classpath of the configuration
> + � � * @param dependencyNode � � � �Class and Service parent ids
> + � � * @param allServiceParents � � ordered list of transitive closure of
> service parents for gbean searches
> + � � * @param attributeStore � � � �Customization info for gbeans
> � � �* @param configurationResolver (there should be a better way) Where
> this configuration is actually located in file system
> + � � * @param kernel
> � � �* @throws InvalidConfigException if this configuration turns out to
> have a problem.
> � � �*/
> � � public Configuration(
> -// � � � � � �@ParamAttribute(name = "classLoaderHolder") ClassLoaderHolder
> classLoaderHolder,
> � � � � � � @ParamAttribute(name = "configurationData") ConfigurationData
> configurationData,
> � � � � � � @ParamAttribute(name = "dependencyNode") DependencyNode
> dependencyNode,
> � � � � � � @ParamAttribute(name = "allServiceParents") List<Configuration>
> allServiceParents,
> � � � � � � @ParamAttribute(name = "attributeStore")
> ManageableAttributeStore attributeStore,
> - � � � � � �@ParamAttribute(name = "configurationResolver")
> ConfigurationResolver configurationResolver) throws InvalidConfigException {
> -// � � � �if (classLoaderHolder == null) throw new
> NullPointerException("classLoaders are null");
> + � � � � � �@ParamAttribute(name = "configurationResolver")
> ConfigurationResolver configurationResolver,
> + � � � � � �@ParamSpecial(type = SpecialAttributeType.kernel) Kernel
> kernel) throws InvalidConfigException {
> � � � � if (configurationData == null) throw new
> NullPointerException("configurationData is null");
>
> �// � � � �this.classLoaderHolder = classLoaderHolder;
> @@ -199,7 +204,7 @@
>
> � � � � � � // Deserialize the GBeans in the configurationData
> � � � � � � Bundle bundle =
> configurationData.getBundleContext().getBundle();
> - � � � � � �Collection<GBeanData> gbeans =
> configurationData.getGBeans(bundle);
> + � � � � � �Collection<GBeanData> gbeans =
> configurationData.getGBeans(bundle, kernel);
> � � � � � � if (attributeStore != null) {
> � � � � � � � � gbeans =
> attributeStore.applyOverrides(dependencyNode.getId(), gbeans, bundle);
> � � � � � � }
> @@ -221,6 +226,7 @@
>
> � � /**
> � � �* Add a contained configuration, such as for a war inside an ear
> + � � *
> � � �* @param child contained configuration
> � � �*/
> � � void addChild(Configuration child) {
> @@ -311,6 +317,7 @@
>
> � � /**
> � � �* Provide a way to locate where this configuration is for web apps and
> persistence units
> + � � *
> � � �* @return the ConfigurationResolver for this configuration
> � � �*/
> � � public ConfigurationResolver getConfigurationResolver() {
> Index: src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
> ===================================================================
> --- src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
> (revision 901109)
> +++ src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
> (working copy)
> @@ -20,6 +20,7 @@
>
> �import org.apache.geronimo.gbean.GBeanData;
> �import org.apache.geronimo.gbean.GBeanInfo;
> +import org.apache.geronimo.kernel.Kernel;
> �import org.apache.geronimo.kernel.Naming;
> �import org.apache.geronimo.kernel.repository.Environment;
> �import org.osgi.framework.Bundle;
> @@ -28,7 +29,7 @@
> �* @version $Rev$ $Date$
> �*/
> �public interface GBeanState {
> - � �List<GBeanData> getGBeans(Bundle bundle) throws InvalidConfigException;
> + � �List<GBeanData> getGBeans(Bundle bundle, Kernel kernel) throws
> InvalidConfigException;
>
> � � void addGBean(GBeanData gbeanData);
>
> Index:
> src/main/java/org/apache/geronimo/kernel/config/SerializedGBeanState.java
> ===================================================================
> ---
> src/main/java/org/apache/geronimo/kernel/config/SerializedGBeanState.java
> (revision 901109)
> +++
> src/main/java/org/apache/geronimo/kernel/config/SerializedGBeanState.java
> (working copy)
> @@ -31,6 +31,7 @@
> �import org.apache.geronimo.gbean.AbstractName;
> �import org.apache.geronimo.gbean.GBeanData;
> �import org.apache.geronimo.gbean.GBeanInfo;
> +import org.apache.geronimo.kernel.Kernel;
> �import org.apache.geronimo.kernel.Naming;
> �import org.apache.geronimo.kernel.ObjectInputStreamExt;
> �import org.apache.geronimo.kernel.repository.Environment;
> @@ -58,11 +59,11 @@
> � � � � }
> � � }
>
> - � �public List<GBeanData> getGBeans(Bundle bundle) throws
> InvalidConfigException {
> + � �public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel) throws
> InvalidConfigException {
> � � � � if (gbeanState == null) {
> � � � � � � return Collections.unmodifiableList(gbeans);
> � � � � }
> - � � � �gbeans.addAll(loadGBeans(gbeanState, bundle));
> + � � � �gbeans.addAll(loadGBeans(gbeanState, bundle, kernel));
> � � � � return Collections.unmodifiableList(gbeans);
> � � }
>
> @@ -110,7 +111,7 @@
> � � � � stream.defaultWriteObject();
> � � }
>
> - � �private static List<GBeanData> loadGBeans(byte[] gbeanState, Bundle
> bundle) throws InvalidConfigException {
> + � �private static List<GBeanData> loadGBeans(byte[] gbeanState, Bundle
> bundle, Kernel kernel) throws InvalidConfigException {
> � � � � List<GBeanData> gbeans = new ArrayList<GBeanData>();
> � � � � if (gbeanState != null && gbeanState.length > 0) {
> � � � � � � // Set the thread context classloader so deserializing classes
> can grab the cl from the thread
> @@ -118,7 +119,7 @@
> � � � � � � try {
> �//
> �Thread.currentThread().setContextClassLoader(classLoader);
>
> - � � � � � � � �ObjectInputStream ois = new ObjectInputStreamExt(new
> ByteArrayInputStream(gbeanState), bundle);
> + � � � � � � � �ObjectInputStream ois = new ObjectInputStreamExt(new
> ByteArrayInputStream(gbeanState), bundle, kernel);
> � � � � � � � � try {
> � � � � � � � � � � while (true) {
> � � � � � � � � � � � � GBeanData gbeanData = new GBeanData();
> Index:
> src/main/java/org/apache/geronimo/kernel/config/SimpleConfigurationManager.java
> ===================================================================
> ---
> src/main/java/org/apache/geronimo/kernel/config/SimpleConfigurationManager.java
> � � (revision 901109)
> +++
> src/main/java/org/apache/geronimo/kernel/config/SimpleConfigurationManager.java
> � � (working copy)
> @@ -424,7 +424,7 @@
>
> � � � � List<Configuration> allServiceParents = buildAllServiceParents(null,
> dependencyNode);
>
> - � � � �Configuration configuration = new Configuration(configurationData,
> dependencyNode, allServiceParents, null, configurationResolver);
> + � � � �Configuration configuration = new Configuration(configurationData,
> dependencyNode, allServiceParents, null, configurationResolver, null);
> � � � � configuration.doStart();
> � � � � //TODO why???
> � � � � resolvedParentIds.add(configuration.getId());
> @@ -1478,7 +1478,8 @@
> � � �*/
> � � private void applyOverrides(Configuration configuration) throws
> InvalidConfigException {
> � � � � Bundle bundle = configuration.getBundle();
> - � � � �Collection<GBeanData> gbeans =
> configuration.getConfigurationData().getGBeans(bundle);
> + � � � �//TODO OSGI we might need a kernel here?
> + � � � �Collection<GBeanData> gbeans =
> configuration.getConfigurationData().getGBeans(bundle, null);
> � � � � if (configuration.getManageableAttributeStore() != null) {
>
> configuration.getManageableAttributeStore().applyOverrides(configuration.getId(),
> gbeans,
> � � � � � � � � � � bundle);
> Index: src/main/java/org/apache/geronimo/gbean/GBeanData.java
> ===================================================================
> --- src/main/java/org/apache/geronimo/gbean/GBeanData.java � � �(revision
> 901109)
> +++ src/main/java/org/apache/geronimo/gbean/GBeanData.java � � �(working
> copy)
> @@ -28,6 +28,7 @@
> �import java.util.Set;
>
> �import org.apache.geronimo.gbean.annotation.EncryptionSetting;
> +import org.apache.geronimo.kernel.ObjectInputStreamExt;
> �import org.apache.geronimo.kernel.repository.Artifact;
>
> �/**
> @@ -419,6 +420,9 @@
>
> � � � � protected void readClassSource(ObjectInput in) throws
> ClassNotFoundException, IOException {
> � � � � � � classSource = (Artifact) in.readObject();
> + � � � � � �if (in instanceof ObjectInputStreamExt) {
> + � � � � � � � �((ObjectInputStreamExt)in).setClassSource(classSource);
> + � � � � � �}
> � � � � }
> � � }
>
>
>

Re: rfc66 fun

Posted by David Jencks <da...@yahoo.com>.
On Jan 20, 2010, at 11:38 AM, Jarek Gawor wrote:

> Hey,
>
> For last few days I've been looking into getting the rfc66 extender
> going in Geronimo but I ran into a problem. As previously mentioned on
> this list, the idea for the extender was to call the Tomcat/Jetty
> ModuleBuilders with a bundle as an input and once the deployment
> process was done start the generated configuration. All without
> creating any additional temporary or permanent bundles.
>
> Because we don't have a single classloader that can load all the
> gbeans within the configuration, David Jencks added a special
> "classSource" attribute to GBeanData which can be used to figure out
> the right Bundle to load the gbean class. Now, since we use Java
> serialization to save and load gbeans, we actually need to set the
> right classloader when we deserialize the gbean. That is, during
> deserialization as soon as we read the "classSource" we need to lookup
> and set the right classloader and then read the rest of the gbean
> data. This should (and seems to) work when all the attributes of the
> gbean are accessible from the same classloader. But what about if the
> gbean has some attribute with some values from different classloaders?
> For example, the Map object we build for jndi context can contain
> objects from different classloaders. I'm not exactly sure what to do
> about it. Although maybe having a custom ObjectOutputStream with
> annotateClass() method that saves "classSource" type of info for each
> unique class might work. Ideas?
>
> Also, I'm dealing with lots of classloader issues since there is no
> single classloader that load all the gbean classes and module classes.
> A lot of Geronimo and other code assumes a single classloader and
> resolving these problems is time consuming and not very fun (although
> probably good in long term). So I'm wondering if we can still somehow
> assemble a single classloader in the extender. For example, the
> http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
> post shows a classloader that delegates a number of different bundles.
> Maybe we could use that in Geronimo especially since we can figure out
> the bundles needed from the configuration environment information.
>
> Thoughts?
>
> Jarek

I think that we are either going to load the gbean from the  
classSource bundle or from the configuration's bundle.  Maybe  
sometimes we need to mix them... not sure.  Anyway I think we can  
modify the ObjectInputStreamExt to look in the classSource's bundle or  
in the bundle as in the attached patch.  I chatted with Jarek a bit on  
irc and I think he is thinking of writing the bundle symbolic name  
into the output stream and using package admin to look it up again.   
I'm a little worried with this approach that we may be mixing too much  
osgi into geronimo.  If we had a blueprint namespace handler I think  
we'd be able to load the classes in the namespace handler rather than  
needing to rely on something that is really like requireBundle.  So,  
my approach is somewhat similar but uses geronimo techniques rather  
than osgi techniques.  I think this might be appropriate since we are  
in fact loading gbeans.

my patch compiles but I haven't tried it yet.

thoughts?

david jencks

Index: src/test/java/org/apache/geronimo/kernel/mock/ 
MockConfigurationManager.java
===================================================================
--- src/test/java/org/apache/geronimo/kernel/mock/ 
MockConfigurationManager.java (revision 901109)
+++ src/test/java/org/apache/geronimo/kernel/mock/ 
MockConfigurationManager.java (working copy)
@@ -122,7 +122,7 @@
      public LifecycleResults loadConfiguration(ConfigurationData  
configurationData, LifecycleMonitor monitor) throws  
NoSuchConfigException, LifecycleException {
          try {
              Artifact configId =  
configurationData.getEnvironment().getConfigId();
-            Configuration configuration = new  
Configuration(configurationData, new DependencyNode(configId, null,  
null), null, null, null);
+            Configuration configuration = new  
Configuration(configurationData, new DependencyNode(configId, null,  
null), null, null, null, null);
              configurations.put(configId, configuration);
          } catch (InvalidConfigException e) {

Index: src/test/java/org/apache/geronimo/kernel/config/ 
ConfigurationUtilTest.java
===================================================================
--- src/test/java/org/apache/geronimo/kernel/config/ 
ConfigurationUtilTest.java  (revision 901109)
+++ src/test/java/org/apache/geronimo/kernel/config/ 
ConfigurationUtilTest.java  (working copy)
@@ -74,10 +74,10 @@

      private void assertEquals(ConfigurationData data,  
ConfigurationData configurationData) throws InvalidConfigException {
          List gbeans;
-        gbeans = data.getGBeans(bundleContext.getBundle());
+        gbeans = data.getGBeans(bundleContext.getBundle(), null);
          assertEquals(configurationData.getId(), data.getId());
          ConfigurationData data3 = (ConfigurationData)  
data.getChildConfigurations().get("testmodule");
-        gbeans = data3.getGBeans(bundleContext.getBundle());
+        gbeans = data3.getGBeans(bundleContext.getBundle(), null);
          assertEquals(new QName("namespaceURI", "localPart"),  
((GBeanData)gbeans.get(0)).getAttribute("someObject"));
      }

Index: src/main/java/org/apache/geronimo/kernel/ 
ObjectInputStreamExt.java
===================================================================
--- src/main/java/org/apache/geronimo/kernel/ 
ObjectInputStreamExt.java  (revision 901109)
+++ src/main/java/org/apache/geronimo/kernel/ 
ObjectInputStreamExt.java  (working copy)
@@ -22,6 +22,9 @@
  import java.io.ObjectStreamClass;
  import java.lang.reflect.Proxy;

+import org.apache.geronimo.kernel.config.Configuration;
+import org.apache.geronimo.kernel.config.InvalidConfigException;
+import org.apache.geronimo.kernel.repository.Artifact;
  import org.osgi.framework.Bundle;

  /**
@@ -29,19 +32,41 @@
   */
  public class ObjectInputStreamExt extends ObjectInputStream {

-    private Bundle bundle;
+    private final Bundle bundle;
+    private final Kernel kernel;
+    private Bundle sourceBundle;

-    public ObjectInputStreamExt(InputStream in, Bundle bundle) throws  
IOException {
+    public ObjectInputStreamExt(InputStream in, Bundle bundle, Kernel  
kernel) throws IOException {
          super(in);
          this.bundle = bundle;
+        this.kernel = kernel;
      }

+    public void setClassSource(Artifact classSource) throws  
ClassNotFoundException {
+        if (classSource != null) {
+            try {
+                Configuration configuration = (Configuration)  
kernel 
.getGBean(Configuration.getConfigurationAbstractName(classSource));
+                sourceBundle = configuration.getBundle();
+            } catch (GBeanNotFoundException e) {
+                throw new ClassNotFoundException("Could not locate  
Configuration to deserialize", e);
+            } catch (InvalidConfigException e) {
+                throw new ClassNotFoundException("Could not identify  
Configuration to deserialize", e);
+            }
+        } else {
+            sourceBundle = null;
+        }
+    }
+
      protected Class resolveClass(ObjectStreamClass classDesc) throws  
IOException, ClassNotFoundException {
+        if (sourceBundle != null) {
+            return ClassLoading.loadClass(classDesc.getName(), bundle);
+        }
          return ClassLoading.loadClass(classDesc.getName(), bundle);
      }

  //see http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
      //currently broken
+
      protected Class resolveProxyClass(String[] interfaces) throws  
IOException, ClassNotFoundException {
          Class[] cinterfaces = new Class[interfaces.length];
          for (int i = 0; i < interfaces.length; i++)
Index: src/main/java/org/apache/geronimo/kernel/config/ 
ConfigurationData.java
===================================================================
--- src/main/java/org/apache/geronimo/kernel/config/ 
ConfigurationData.java      (revision 901109)
+++ src/main/java/org/apache/geronimo/kernel/config/ 
ConfigurationData.java      (working copy)
@@ -28,6 +28,7 @@

  import org.apache.geronimo.gbean.GBeanData;
  import org.apache.geronimo.gbean.GBeanInfo;
+import org.apache.geronimo.kernel.Kernel;
  import org.apache.geronimo.kernel.Naming;
  import org.apache.geronimo.kernel.repository.Artifact;
  import org.apache.geronimo.kernel.repository.Environment;
@@ -179,9 +180,9 @@
          return environment.getManifest();
      }

-    public List<GBeanData> getGBeans(Bundle bundle) throws  
InvalidConfigException {
+    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)  
throws InvalidConfigException {
          if (bundle == null) throw new NullPointerException("bundle  
is null");
-        List<GBeanData> gbeans = gbeanState.getGBeans(bundle);
+        List<GBeanData> gbeans = gbeanState.getGBeans(bundle, kernel);
          if (null == configurationDataTransformer) {
              return gbeans;
          }
Index: src/main/java/org/apache/geronimo/kernel/config/xstream/ 
XStreamGBeanState.java
===================================================================
--- src/main/java/org/apache/geronimo/kernel/config/xstream/ 
XStreamGBeanState.java      (revision 901109)
+++ src/main/java/org/apache/geronimo/kernel/config/xstream/ 
XStreamGBeanState.java      (working copy)
@@ -32,6 +32,7 @@
  import org.apache.geronimo.gbean.AbstractName;
  import org.apache.geronimo.gbean.GBeanData;
  import org.apache.geronimo.gbean.GBeanInfo;
+import org.apache.geronimo.kernel.Kernel;
  import org.apache.geronimo.kernel.Naming;
  import org.apache.geronimo.kernel.util.XmlUtil;
  import org.apache.geronimo.kernel.config.InvalidConfigException;
@@ -73,7 +74,7 @@
          return gbeanState;
      }

-    public List<GBeanData> getGBeans(Bundle bundle) throws  
InvalidConfigException {
+    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)  
throws InvalidConfigException {
          if (gbeanState == null) {
              return Collections.unmodifiableList(gbeans);
          }
Index: src/main/java/org/apache/geronimo/kernel/config/ 
Configuration.java
===================================================================
--- src/main/java/org/apache/geronimo/kernel/config/ 
Configuration.java  (revision 901109)
+++ src/main/java/org/apache/geronimo/kernel/config/ 
Configuration.java  (working copy)
@@ -38,8 +38,11 @@
  import org.apache.geronimo.gbean.ReferencePatterns;
  import org.apache.geronimo.gbean.annotation.GBean;
  import org.apache.geronimo.gbean.annotation.ParamAttribute;
+import org.apache.geronimo.gbean.annotation.ParamSpecial;
+import org.apache.geronimo.gbean.annotation.SpecialAttributeType;
  import org.apache.geronimo.kernel.GBeanAlreadyExistsException;
  import org.apache.geronimo.kernel.GBeanNotFoundException;
+import org.apache.geronimo.kernel.Kernel;
  import org.apache.geronimo.kernel.Naming;
  import org.apache.geronimo.kernel.repository.Artifact;
  import org.apache.geronimo.kernel.repository.Environment;
@@ -86,6 +89,7 @@
       * Converts an Artifact to an AbstractName for a configuration.   
Does not
       * validate that this is a reasonable or resolved Artifact, or  
that it
       * corresponds to an actual Configuration.
+     *
       * @param configId id for configuration
       * @return abstract name constructed from supplied id
       * @throws InvalidConfigException if the ObjectName could not be  
constructed
@@ -167,23 +171,24 @@

      /**
       * Creates a configuration.
+     * <p/>
+     * //     * @param classLoaderHolder Classloaders for this  
configuration
       *
-//     * @param classLoaderHolder Classloaders for this configuration
-     * @param configurationData the module type, environment and  
classpath of the configuration
-     * @param dependencyNode Class and Service parent ids
-     * @param allServiceParents ordered list of transitive closure of  
service parents for gbean searches
-     * @param attributeStore Customization info for gbeans
+     * @param configurationData     the module type, environment and  
classpath of the configuration
+     * @param dependencyNode        Class and Service parent ids
+     * @param allServiceParents     ordered list of transitive  
closure of service parents for gbean searches
+     * @param attributeStore        Customization info for gbeans
       * @param configurationResolver (there should be a better way)  
Where this configuration is actually located in file system
+     * @param kernel
       * @throws InvalidConfigException if this configuration turns  
out to have a problem.
       */
      public Configuration(
-//            @ParamAttribute(name = "classLoaderHolder")  
ClassLoaderHolder classLoaderHolder,
              @ParamAttribute(name = "configurationData")  
ConfigurationData configurationData,
              @ParamAttribute(name = "dependencyNode") DependencyNode  
dependencyNode,
              @ParamAttribute(name = "allServiceParents")  
List<Configuration> allServiceParents,
              @ParamAttribute(name = "attributeStore")  
ManageableAttributeStore attributeStore,
-            @ParamAttribute(name = "configurationResolver")  
ConfigurationResolver configurationResolver) throws  
InvalidConfigException {
-//        if (classLoaderHolder == null) throw new  
NullPointerException("classLoaders are null");
+            @ParamAttribute(name = "configurationResolver")  
ConfigurationResolver configurationResolver,
+            @ParamSpecial(type = SpecialAttributeType.kernel) Kernel  
kernel) throws InvalidConfigException {
          if (configurationData == null) throw new  
NullPointerException("configurationData is null");

  //        this.classLoaderHolder = classLoaderHolder;
@@ -199,7 +204,7 @@

              // Deserialize the GBeans in the configurationData
              Bundle bundle =  
configurationData.getBundleContext().getBundle();
-            Collection<GBeanData> gbeans =  
configurationData.getGBeans(bundle);
+            Collection<GBeanData> gbeans =  
configurationData.getGBeans(bundle, kernel);
              if (attributeStore != null) {
                  gbeans =  
attributeStore.applyOverrides(dependencyNode.getId(), gbeans, bundle);
              }
@@ -221,6 +226,7 @@

      /**
       * Add a contained configuration, such as for a war inside an ear
+     *
       * @param child contained configuration
       */
      void addChild(Configuration child) {
@@ -311,6 +317,7 @@

      /**
       * Provide a way to locate where this configuration is for web  
apps and persistence units
+     *
       * @return the ConfigurationResolver for this configuration
       */
      public ConfigurationResolver getConfigurationResolver() {
Index: src/main/java/org/apache/geronimo/kernel/config/GBeanState.java
===================================================================
--- src/main/java/org/apache/geronimo/kernel/config/ 
GBeanState.java     (revision 901109)
+++ src/main/java/org/apache/geronimo/kernel/config/ 
GBeanState.java     (working copy)
@@ -20,6 +20,7 @@

  import org.apache.geronimo.gbean.GBeanData;
  import org.apache.geronimo.gbean.GBeanInfo;
+import org.apache.geronimo.kernel.Kernel;
  import org.apache.geronimo.kernel.Naming;
  import org.apache.geronimo.kernel.repository.Environment;
  import org.osgi.framework.Bundle;
@@ -28,7 +29,7 @@
   * @version $Rev$ $Date$
   */
  public interface GBeanState {
-    List<GBeanData> getGBeans(Bundle bundle) throws  
InvalidConfigException;
+    List<GBeanData> getGBeans(Bundle bundle, Kernel kernel) throws  
InvalidConfigException;

      void addGBean(GBeanData gbeanData);

Index: src/main/java/org/apache/geronimo/kernel/config/ 
SerializedGBeanState.java
===================================================================
--- src/main/java/org/apache/geronimo/kernel/config/ 
SerializedGBeanState.java   (revision 901109)
+++ src/main/java/org/apache/geronimo/kernel/config/ 
SerializedGBeanState.java   (working copy)
@@ -31,6 +31,7 @@
  import org.apache.geronimo.gbean.AbstractName;
  import org.apache.geronimo.gbean.GBeanData;
  import org.apache.geronimo.gbean.GBeanInfo;
+import org.apache.geronimo.kernel.Kernel;
  import org.apache.geronimo.kernel.Naming;
  import org.apache.geronimo.kernel.ObjectInputStreamExt;
  import org.apache.geronimo.kernel.repository.Environment;
@@ -58,11 +59,11 @@
          }
      }

-    public List<GBeanData> getGBeans(Bundle bundle) throws  
InvalidConfigException {
+    public List<GBeanData> getGBeans(Bundle bundle, Kernel kernel)  
throws InvalidConfigException {
          if (gbeanState == null) {
              return Collections.unmodifiableList(gbeans);
          }
-        gbeans.addAll(loadGBeans(gbeanState, bundle));
+        gbeans.addAll(loadGBeans(gbeanState, bundle, kernel));
          return Collections.unmodifiableList(gbeans);
      }

@@ -110,7 +111,7 @@
          stream.defaultWriteObject();
      }

-    private static List<GBeanData> loadGBeans(byte[] gbeanState,  
Bundle bundle) throws InvalidConfigException {
+    private static List<GBeanData> loadGBeans(byte[] gbeanState,  
Bundle bundle, Kernel kernel) throws InvalidConfigException {
          List<GBeanData> gbeans = new ArrayList<GBeanData>();
          if (gbeanState != null && gbeanState.length > 0) {
              // Set the thread context classloader so deserializing  
classes can grab the cl from the thread
@@ -118,7 +119,7 @@
              try {
  //                 
Thread.currentThread().setContextClassLoader(classLoader);

-                ObjectInputStream ois = new ObjectInputStreamExt(new  
ByteArrayInputStream(gbeanState), bundle);
+                ObjectInputStream ois = new ObjectInputStreamExt(new  
ByteArrayInputStream(gbeanState), bundle, kernel);
                  try {
                      while (true) {
                          GBeanData gbeanData = new GBeanData();
Index: src/main/java/org/apache/geronimo/kernel/config/ 
SimpleConfigurationManager.java
===================================================================
--- src/main/java/org/apache/geronimo/kernel/config/ 
SimpleConfigurationManager.java     (revision 901109)
+++ src/main/java/org/apache/geronimo/kernel/config/ 
SimpleConfigurationManager.java     (working copy)
@@ -424,7 +424,7 @@

          List<Configuration> allServiceParents =  
buildAllServiceParents(null, dependencyNode);

-        Configuration configuration = new  
Configuration(configurationData, dependencyNode, allServiceParents,  
null, configurationResolver);
+        Configuration configuration = new  
Configuration(configurationData, dependencyNode, allServiceParents,  
null, configurationResolver, null);
          configuration.doStart();
          //TODO why???
          resolvedParentIds.add(configuration.getId());
@@ -1478,7 +1478,8 @@
       */
      private void applyOverrides(Configuration configuration) throws  
InvalidConfigException {
          Bundle bundle = configuration.getBundle();
-        Collection<GBeanData> gbeans =  
configuration.getConfigurationData().getGBeans(bundle);
+        //TODO OSGI we might need a kernel here?
+        Collection<GBeanData> gbeans =  
configuration.getConfigurationData().getGBeans(bundle, null);
          if (configuration.getManageableAttributeStore() != null) {
               
configuration 
.getManageableAttributeStore().applyOverrides(configuration.getId(),  
gbeans,
                      bundle);
Index: src/main/java/org/apache/geronimo/gbean/GBeanData.java
===================================================================
--- src/main/java/org/apache/geronimo/gbean/GBeanData.java       
(revision 901109)
+++ src/main/java/org/apache/geronimo/gbean/GBeanData.java       
(working copy)
@@ -28,6 +28,7 @@
  import java.util.Set;

  import org.apache.geronimo.gbean.annotation.EncryptionSetting;
+import org.apache.geronimo.kernel.ObjectInputStreamExt;
  import org.apache.geronimo.kernel.repository.Artifact;

  /**
@@ -419,6 +420,9 @@

          protected void readClassSource(ObjectInput in) throws  
ClassNotFoundException, IOException {
              classSource = (Artifact) in.readObject();
+            if (in instanceof ObjectInputStreamExt) {
+                ((ObjectInputStreamExt)in).setClassSource(classSource);
+            }
          }
      }



Re: rfc66 fun

Posted by Rick McGuire <ri...@gmail.com>.
On 1/20/2010 2:38 PM, Jarek Gawor wrote:
> Hey,
>
> For last few days I've been looking into getting the rfc66 extender
> going in Geronimo but I ran into a problem. As previously mentioned on
> this list, the idea for the extender was to call the Tomcat/Jetty
> ModuleBuilders with a bundle as an input and once the deployment
> process was done start the generated configuration. All without
> creating any additional temporary or permanent bundles.
>
> Because we don't have a single classloader that can load all the
> gbeans within the configuration, David Jencks added a special
> "classSource" attribute to GBeanData which can be used to figure out
> the right Bundle to load the gbean class. Now, since we use Java
> serialization to save and load gbeans, we actually need to set the
> right classloader when we deserialize the gbean. That is, during
> deserialization as soon as we read the "classSource" we need to lookup
> and set the right classloader and then read the rest of the gbean
> data. This should (and seems to) work when all the attributes of the
> gbean are accessible from the same classloader. But what about if the
> gbean has some attribute with some values from different classloaders?
> For example, the Map object we build for jndi context can contain
> objects from different classloaders. I'm not exactly sure what to do
> about it. Although maybe having a custom ObjectOutputStream with
> annotateClass() method that saves "classSource" type of info for each
> unique class might work. Ideas?
>
> Also, I'm dealing with lots of classloader issues since there is no
> single classloader that load all the gbean classes and module classes.
> A lot of Geronimo and other code assumes a single classloader and
> resolving these problems is time consuming and not very fun (although
> probably good in long term). So I'm wondering if we can still somehow
> assemble a single classloader in the extender. For example, the
> http://www.osgi.org/blog/2008/08/classy-solutions-to-tricky-proxies.html
> post shows a classloader that delegates a number of different bundles.
> Maybe we could use that in Geronimo especially since we can figure out
> the bundles needed from the configuration environment information.
>    
There have been a number of situations while during the conversion that 
I've suspected we were going to need a classloader that could delegate 
to more than one bundle instance as we developed more complex dependency 
situations.  I suspect it's finally time to bit that particular bullet.

Rick
> Thoughts?
>
> Jarek
>
>