Migrated attachment for Guice issue 343, comment 2

GUICE_ISSUE_343_SRC_20090303.txt

Index: src/com/google/inject/internal/BytecodeGen.java
===================================================================
--- src/com/google/inject/internal/BytecodeGen.java	(revision 903)
+++ src/com/google/inject/internal/BytecodeGen.java	(working copy)
@@ -59,8 +59,10 @@
 
   private static final Logger logger = Logger.getLogger(BytecodeGen.class.getName());
 
-  static final ClassLoader GUICE_CLASS_LOADER = BytecodeGen.class.getClassLoader();
+  static final ClassLoader GUICE_CLASS_LOADER = canonicalize(BytecodeGen.class.getClassLoader());
 
+  static final Object NULL_CLASS_LOADER_KEY = new Object();
+
   /** ie. "com.google.inject.internal" */
   private static final String GUICE_INTERNAL_PACKAGE
       = BytecodeGen.class.getName().replaceFirst("\\.internal\\..*$", ".internal");
@@ -89,10 +91,11 @@
    * Weak cache of bridge class loaders that make the Guice implementation
    * classes visible to various code-generated proxies of client classes.
    */
-  private static final Map<ClassLoader, ClassLoader> CLASS_LOADER_CACHE
+  private static final Map<Object, ClassLoader> CLASS_LOADER_CACHE
       = new MapMaker().weakKeys().weakValues().makeComputingMap(
-          new Function<ClassLoader, ClassLoader>() {
-    public ClassLoader apply(final @Nullable ClassLoader typeClassLoader) {
+          new Function<Object, ClassLoader>() {
+    public ClassLoader apply(@Nullable Object classLoaderKey) {
+      final ClassLoader typeClassLoader = getClassLoaderFromKey(classLoaderKey);
       logger.fine("Creating a bridge ClassLoader for " + typeClassLoader);
       return AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
         public ClassLoader run() {
@@ -104,28 +107,35 @@
 
   /**
    * For class loaders, {@code null}, is always an alias to the
-   * {@link ClassLoader#getSystemClassLoader() system class loader}. This method
-   * will not return null.
+   * {@link ClassLoader#getSystemClassLoader() system class loader}.
    */
   private static ClassLoader canonicalize(ClassLoader classLoader) {
-    return classLoader != null
-        ? classLoader
-        : checkNotNull(getSystemClassLoaderOrNull(), "Couldn't get a ClassLoader");
-  }
+    if (classLoader != null) {
+      return classLoader;
+    }
 
-  /**
-   * Returns the system classloader, or {@code null} if we don't have
-   * permission.
-   */
-  private static ClassLoader getSystemClassLoaderOrNull() {
     try {
       return ClassLoader.getSystemClassLoader();
     } catch (SecurityException e) {
-      return null;
+      return null; // unable to canonicalize
     }
   }
 
   /**
+   * Returns a non-null key for the given class loader.
+   */
+  private static Object getKeyFromClassLoader(ClassLoader classLoader) {
+    return classLoader != null ? classLoader : NULL_CLASS_LOADER_KEY;
+  }
+
+  /**
+   * Returns the class loader related to the given key.
+   */
+  private static ClassLoader getClassLoaderFromKey(Object key) {
+    return key != NULL_CLASS_LOADER_KEY ? (ClassLoader)key : null;
+  }
+
+  /**
    * Returns the class loader to host generated classes for {@code type}.
    */
   public static ClassLoader getClassLoader(Class<?> type) {
@@ -135,8 +145,8 @@
   private static ClassLoader getClassLoader(Class<?> type, ClassLoader delegate) {
     delegate = canonicalize(delegate);
 
-    // if the application is running in the System classloader, assume we can run there too
-    if (delegate == getSystemClassLoaderOrNull()) {
+    // simple case, we're running in the same class-space
+    if (GUICE_CLASS_LOADER == delegate) {
       return delegate;
     }
 
@@ -146,7 +156,7 @@
     }
 
     if (HOOK_ENABLED && Visibility.forType(type) == Visibility.PUBLIC) {
-      return CLASS_LOADER_CACHE.get(delegate);
+      return CLASS_LOADER_CACHE.get(getKeyFromClassLoader(delegate));
     }
 
     return delegate;

GUICE_ISSUE_343_TEST_20090303.txt

Index: test/com/googlecode/guice/BytecodeGenTest.java
===================================================================
--- test/com/googlecode/guice/BytecodeGenTest.java	(revision 903)
+++ test/com/googlecode/guice/BytecodeGenTest.java	(working copy)
@@ -43,13 +43,17 @@
 public class BytecodeGenTest extends TestCase {
 
   private final ClassLoader systemClassLoader = ClassLoader.getSystemClassLoader();
+  private final ClassLoader guiceClassLoader = Guice.class.getClassLoader();
 
   private final Module interceptorModule = new AbstractModule() {
     protected void configure() {
       bindInterceptor(any(), any(), new MethodInterceptor() {
         public Object invoke(MethodInvocation chain)
             throws Throwable {
-          return chain.proceed() + " WORLD";
+          if (chain.getMethod().getReturnType() == String.class) {
+            return chain.proceed() + " WORLD";
+          }
+          return chain.proceed();
         }
       });
     }
@@ -183,7 +187,7 @@
     assertEquals("HELLO WORLD", m.invoke(testObject));
   }
 
-  public void testSystemClassLoaderIsUsedIfProxiedClassUsesIt() {
+  public void testGuiceClassLoaderIsUsedIfProxiedClassUsesIt() {
     ProxyTest testProxy = Guice.createInjector(interceptorModule, new Module() {
       public void configure(Binder binder) {
         binder.bind(ProxyTest.class).to(ProxyTestImpl.class);
@@ -191,11 +195,11 @@
     }).getInstance(ProxyTest.class);
 
     // unforunately, the expected classloader depends on which class loader loaded this test.
-    if (ProxyTest.class.getClassLoader() == systemClassLoader) {
-      assertSame(testProxy.getClass().getClassLoader(), systemClassLoader);
+    if (ProxyTest.class.getClassLoader() == guiceClassLoader) {
+      assertSame(testProxy.getClass().getClassLoader(), guiceClassLoader);
     } else {
       assertNotSame(testProxy.getClass().getClassLoader(), ProxyTest.class.getClassLoader());
-      assertNotSame(testProxy.getClass().getClassLoader(), systemClassLoader);
+      assertNotSame(testProxy.getClass().getClassLoader(), guiceClassLoader);
     }
   }
   
@@ -204,6 +208,7 @@
         .getInstance(proxyTestClass);
     assertNotNull(testObject.getClass().getClassLoader());
     assertNotSame(testObject.getClass().getClassLoader(), systemClassLoader);
+    assertNotSame(testObject.getClass().getClassLoader(), guiceClassLoader);
 
     // take a weak reference to the generated proxy class
     Reference<Class<?>> clazzRef = new WeakReference<Class<?>>(testObject.getClass());
@@ -231,6 +236,7 @@
     assertEquals("HI WORLD", injector.getInstance(PackageClassPackageMethod.class).sayHi());
     assertEquals("HI WORLD", injector.getInstance(PublicClassPackageMethod.class).sayHi());
     assertEquals("HI WORLD", injector.getInstance(ProtectedClassProtectedMethod.class).sayHi());
+    assertEquals("[] WORLD", injector.getInstance(java.util.ArrayList.class).toString());
   }
 
   static class PackageClassPackageMethod {