2010/1/21 David Jencks <[email protected]> > > 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 <[email protected]> >> 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
