You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by rb...@gmail.com on 2011/01/15 02:12:51 UTC

The class loaders some of the classes in Shindig use can cause problems in an OSGi enviornment (issue3988042)

Reviewers: dev-remailer_shindig.apache.org, dev_shindig.apache.org,

Description:
For my project I am embedding
Shindig inside an Eclipse based application. To do this we have decided
to modularize Shindig a bit, and separate out the libraries (jars in the
lib folder) Shindig uses into one plugin, and the actual web app into a
different plugin. Once I did this however Shindig could no longer find
any of my guice modules, customized shindig.properties file, or
customized
container.js file living in the web app plugin. In the end I figured out
this was happening because of the class loader some of the Shindig
classes
use. From what I found, it looks like we are either using the current
classes class loader or doing Class.forName(className) to load classes
and
resources. This is fine when a web app is running on Tomcat or Jetty but
in OSGi this can possibly break. In OSGi, the class loader will use the
current bundle's context. So if you have a class foo within a jar in
bundle A using the methods in Shindig today to load a class bar from
bundle B, the class foo will use bundle A's class loader and cannot find
the class bar in bundle B. The easiest way to fix this would be to try
to
use the class loader returned from
Thread.currentThread().getContextClassLoader(), when the current codes
class loader fails to load the class or resource requested. So far I
have
found problems in ResourceLoader.openResource and
GuiceServletContextListener.contextInitialized. The problem in
GuiceServletContextListener I can work around by extending this class
and
overriding contextInitialized, however there is no way for me to work
around the problem in ResourceLoader, so I figured I would make the
change
in GuiceServletContextListener as well.

Please review this at http://codereview.appspot.com/3988042/

Affected files:
    
java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
    
java/common/src/main/java/org/apache/shindig/common/util/ResourceLoader.java


### Eclipse Workspace Patch 1.0
#P shindig-project
Index:  
java/common/src/main/java/org/apache/shindig/common/util/ResourceLoader.java
===================================================================
---  
java/common/src/main/java/org/apache/shindig/common/util/ResourceLoader.java	 
(revision 1058142)
+++  
java/common/src/main/java/org/apache/shindig/common/util/ResourceLoader.java	 
(working copy)
@@ -54,14 +54,34 @@
     * @return An input stream for the given named resource
     * @throws FileNotFoundException
     */
-  public static InputStream openResource(String resource) throws  
IOException {
+  public static InputStream openResource(String resource) throws  
FileNotFoundException  {
      ClassLoader cl = ResourceLoader.class.getClassLoader();
-    InputStream is = cl.getResourceAsStream(resource.trim());
-    if (is == null) {
-      throw new FileNotFoundException("Can not locate resource: " +  
resource);
-    }
+    InputStream is;
+	try {
+		is = openResource(cl, resource);
+	} catch (FileNotFoundException e) {
+		//If we cannot find the resource using the current classes class loader
+		//try the current threads
+		cl = Thread.currentThread().getContextClassLoader();
+		is = openResource(cl, resource);
+	}
      return is;
    }
+
+  /**
+   * Opens a resource
+   * @param cl The classloader to use to find the resource
+   * @param resource The resource to open
+   * @return An input stream for the given named resource
+   * @throws FileNotFoundException
+   */
+  public static InputStream openResource(ClassLoader cl, String resource)  
throws FileNotFoundException{
+	  InputStream is = cl.getResourceAsStream(resource.trim());
+	  if(is == null){
+		  throw new FileNotFoundException("Can not locate resource: " +  
resource);
+	  }
+	  return is;
+  }

    /**
     * Reads the contents of a resource as a string.
Index:  
java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
===================================================================
---  
java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java	 
(revision 1058142)
+++  
java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java	 
(working copy)
@@ -48,31 +48,38 @@
    private static final String SYSTEM_PROPERTIES = "system.properties";

    public void contextInitialized(ServletContextEvent event) {
-    ServletContext context = event.getServletContext();
-    //HNN setting all system.properties specified in the web.xml
-    setSystemProperties(context);
-    String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
-    List<Module> modules = Lists.newLinkedList();
-    if (moduleNames != null) {
-      for (String moduleName : StringUtils.split(moduleNames, ':')) {
-        try {
-          moduleName = moduleName.trim();
-          if (moduleName.length() > 0) {
-            modules.add((Module)Class.forName(moduleName).newInstance());
-          }
-        } catch (InstantiationException e) {
-          throw new RuntimeException(e);
-        } catch (ClassNotFoundException e) {
-          throw new RuntimeException(e);
-        } catch (IllegalAccessException e) {
-          throw new RuntimeException(e);
-        }
-      }
-    }
-    Injector injector = Guice.createInjector(Stage.PRODUCTION, modules);
-    context.setAttribute(INJECTOR_ATTRIBUTE, injector);
+	    ServletContext context = event.getServletContext();
+	    //HNN setting all system.properties specified in the web.xml
+	    setSystemProperties(context);
+	    String moduleNames = context.getInitParameter(MODULES_ATTRIBUTE);
+	    List<Module> modules = Lists.newLinkedList();
+	    if (moduleNames != null) {
+	      for (String moduleName : StringUtils.split(moduleNames, ':')) {
+	        try {
+	          moduleName = moduleName.trim();
+	          if (moduleName.length() > 0) {
+	        	  try{
+	        		  modules.add((Module)Class.forName(moduleName).newInstance());
+	        	  } catch (Throwable t){
+	        		  //If we cannot find the class using forName try the current
+	        		  //threads class loader
+	        		   
modules.add((Module)Thread.currentThread().getContextClassLoader().loadClass(moduleName).newInstance());
+	        	  }
+	          }
+	        } catch (InstantiationException e) {
+	          throw new RuntimeException(e);
+	        } catch (ClassNotFoundException e) {
+	          throw new RuntimeException(e);
+	        } catch (IllegalAccessException e) {
+	          throw new RuntimeException(e);
+	        }
+	      }
+	    }
+	    Injector injector = Guice.createInjector(Stage.PRODUCTION, modules);
+	    context.setAttribute(INJECTOR_ATTRIBUTE, injector);

-  }
+	  }
+

    public void contextDestroyed(ServletContextEvent event) {
      ServletContext context = event.getServletContext();