Patches to let Guice be more OSGi (and classloader) friendly. Solves issue 439, issue 337, issue 443, and issue 343. All provided by Stuart McCulloch. Many thanks, Stuart!

git-svn-id: https://google-guice.googlecode.com/svn/trunk@1158 d779f126-a31b-0410-b53b-1d3aecad763e

24 files changed, 787 insertions, 69 deletions

diff --git a/build.properties b/build.properties
index 239bcb9a..d44c958c 100644
--- a/build.properties
+++ b/build.properties
@@ -26,4 +26,4 @@ javadoc.packagenames=com.google.inject,com.google.inject.spi,\
   com.google.inject.util
 test.class=com.google.inject.AllTests
 module=com.google.inject
-exclude.imports: !net.sf.cglib.*,!org.objectweb.asm.*
+imports=!net.sf.cglib.*,!org.objectweb.asm.*
diff --git a/build.xml b/build.xml
index 659a7d47..6b436333 100644
--- a/build.xml
+++ b/build.xml
@@ -2,9 +2,6 @@
 
 <project name="guice" default="compile">
 
-  <property name="DynamicImport-Package" value="org.aopalliance.intercept"/>
-  <property name="exclude.imports" value="!com.google.common.*,!net.sf.*,!org.objectweb.*"/>
-
   <import file="common.xml"/>
 
   <path id="compile.classpath">
@@ -90,10 +87,15 @@
         <pathelement location="lib/build/easymock.jar"/>
         <pathelement location="lib/javax.inject.jar"/>
         <pathelement location="lib/build/javax.inject-tck.jar"/>
+        <pathelement location="lib/build/bnd-0.0.384.jar"/>
+        <pathelement location="lib/build/felix-2.0.5.jar"/>
       </classpath>
       <arg value="com.google.inject.AllTests"/>    
       <syspropertyset>
         <propertyref name="guice.custom.loader"/>
+        <propertyref name="version"/>
+        <propertyref name="build.dir"/>
+        <propertyref name="lib.dir"/>
       </syspropertyset>
     </java>
   </target>
diff --git a/common.xml b/common.xml
index da6269c0..fa27d0e7 100644
--- a/common.xml
+++ b/common.xml
@@ -7,7 +7,7 @@
   <!-- can be overridden at the command line with -Dversion=
        or in IDEA, in the ant properties dialog -->
   <property name="version" value="snapshot"/>
-  <property name="api.version" value="2.1"/>
+  <property name="api.version" value="1.3"/>
 
   <target name="compile" description="Compile Java source.">
     <mkdir dir="${build.dir}/classes"/>
@@ -25,13 +25,14 @@
   <target name="manifest" description="Generate OSGi manifest." depends="compile">
     <dirname property="common.basedir" file="${ant.file.common}"/>
     <taskdef resource="aQute/bnd/ant/taskdef.properties"
-      classpath="${common.basedir}/lib/build/bnd-0.0.305.jar"/>
+      classpath="${common.basedir}/lib/build/bnd-0.0.384.jar"/>
 
     <fail unless="module" message="Missing 'module' property (use the primary package name in this jar)"/>
+    <property name="imports" value=""/>
 
     <property name="Bundle-Name" value="${ant.project.name}"/>
     <property name="Bundle-SymbolicName" value="${module}"/>
-    <property name="Bundle-Version" value="${replace;${version};^[^0-9];${api.version}.$0}"/>
+    <property name="Bundle-Version" value="${replace;${version};^[^0-9];0.0.0.$0}"/>
 
     <property name="Bundle-Description" value="Guice is a lightweight dependency injection framework for Java 5 and above"/>
     <property name="Bundle-DocURL" value="http://code.google.com/p/google-guice/"/>
@@ -39,12 +40,18 @@
     <property name="Bundle-License" value="http://www.apache.org/licenses/LICENSE-2.0"/>
     <property name="Bundle-Vendor" value="Google Inc."/>
 
-    <property name="exclude.imports" value=""/>
-    <property name="api.range" value="&quot;[${api.version},${version;+;${api.version}})&quot;"/>
-    <property name="guice.imports" value="com.google.inject.*;version=${api.range}"/>
-    <property name="Import-Package" value="${exclude.imports},${guice.imports},*;resolution:=optional"/>
     <property name="Export-Package" value="!${module}.internal.*,${module}.*;version=${api.version}"/>
 
+    <condition property="Import-Package" value="!com.google.inject.*,*" else="!${module}.*,${imports},*">
+      <istrue value="${fragment}"/>
+    </condition>
+
+    <condition property="Fragment-Host" value="com.google.inject">
+      <istrue value="${fragment}"/>
+    </condition>
+
+    <property name="-nouses" value="true"/>
+
     <property name="-removeheaders" value="Bnd-LastModified,Ignore-Package,Include-Resource,Private-Package,Tool"/>
 
     <bndwrap jars="${build.dir}/classes" output="${build.dir}"/>
diff --git a/extensions/assistedinject/build.properties b/extensions/assistedinject/build.properties
index e1f60a37..9dce2f6f 100644
--- a/extensions/assistedinject/build.properties
+++ b/extensions/assistedinject/build.properties
@@ -4,3 +4,4 @@ test.dir=test
 build.dir=build
 test.class=com.google.inject.assistedinject.FactoryProviderTest
 module=com.google.inject.assistedinject
+fragment=true
diff --git a/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider.java b/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider.java
index 31342797..deab85ff 100755
--- a/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider.java
+++ b/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider.java
@@ -23,6 +23,7 @@ import com.google.inject.Key;
 import com.google.inject.Provider;
 import com.google.inject.TypeLiteral;
 import static com.google.inject.internal.Annotations.getKey;
+import com.google.inject.internal.BytecodeGen;
 import com.google.inject.internal.Errors;
 import com.google.inject.internal.ErrorsException;
 import com.google.inject.internal.ImmutableMap;
@@ -365,7 +366,7 @@ public class FactoryProvider<F> implements Provider<F>, HasDependencies {
 
     @SuppressWarnings("unchecked") // we imprecisely treat the class literal of T as a Class<T>
     Class<F> factoryRawType = (Class) factoryType.getRawType();
-    return factoryRawType.cast(Proxy.newProxyInstance(factoryRawType.getClassLoader(),
+    return factoryRawType.cast(Proxy.newProxyInstance(BytecodeGen.getClassLoader(factoryRawType),
         new Class[] { factoryRawType }, invocationHandler));
   }
 
diff --git a/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider2.java b/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider2.java
index b6350cef..d3e27838 100644
--- a/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider2.java
+++ b/extensions/assistedinject/src/com/google/inject/assistedinject/FactoryProvider2.java
@@ -28,6 +28,7 @@ import com.google.inject.Provider;
 import com.google.inject.ProvisionException;
 import com.google.inject.TypeLiteral;
 import static com.google.inject.internal.Annotations.getKey;
+import com.google.inject.internal.BytecodeGen;
 import com.google.inject.internal.Errors;
 import com.google.inject.internal.ErrorsException;
 import com.google.inject.internal.ImmutableList;
@@ -214,7 +215,7 @@ final class FactoryProvider2<F> implements InvocationHandler, Provider<F> {
       throw new ConfigurationException(e.getErrors().getMessages());
     }
     
-    factory = factoryRawType.cast(Proxy.newProxyInstance(factoryRawType.getClassLoader(),
+    factory = factoryRawType.cast(Proxy.newProxyInstance(BytecodeGen.getClassLoader(factoryRawType),
         new Class[] { factoryRawType }, this));
   }
 
diff --git a/extensions/grapher/build.properties b/extensions/grapher/build.properties
index 980684c4..1461d3cd 100644
--- a/extensions/grapher/build.properties
+++ b/extensions/grapher/build.properties
@@ -4,3 +4,4 @@ test.dir=test
 build.dir=build
 test.class=com.google.inject.grapher.AllTests
 module=com.google.inject.grapher
+fragment=true
diff --git a/extensions/multibindings/build.properties b/extensions/multibindings/build.properties
index 9575ddb1..77228a85 100644
--- a/extensions/multibindings/build.properties
+++ b/extensions/multibindings/build.properties
@@ -4,3 +4,4 @@ test.dir=test
 build.dir=build
 test.class=com.google.inject.multibindings.AllTests
 module=com.google.inject.multibindings
+fragment=true
diff --git a/extensions/throwingproviders/build.properties b/extensions/throwingproviders/build.properties
index abcc096a..3b5d5a84 100644
--- a/extensions/throwingproviders/build.properties
+++ b/extensions/throwingproviders/build.properties
@@ -4,3 +4,4 @@ test.dir=test
 build.dir=build
 test.class=com.google.inject.throwingproviders.ThrowingProviderBinderTest
 module=com.google.inject.throwingproviders
+fragment=true
diff --git a/lib/build/bnd-0.0.305.jar b/lib/build/bnd-0.0.305.jar
deleted file mode 100644
index 21306062..00000000
--- a/lib/build/bnd-0.0.305.jar
+++ /dev/null
diff --git a/lib/build/bnd-0.0.384.jar b/lib/build/bnd-0.0.384.jar
new file mode 100644
index 00000000..fa5e5263
--- /dev/null
+++ b/lib/build/bnd-0.0.384.jar
diff --git a/lib/build/felix-2.0.5.jar b/lib/build/felix-2.0.5.jar
new file mode 100644
index 00000000..71e5a842
--- /dev/null
+++ b/lib/build/felix-2.0.5.jar
diff --git a/lifecycle/build.properties b/lifecycle/build.properties
index c0a2be42..b5142056 100644
--- a/lifecycle/build.properties
+++ b/lifecycle/build.properties
@@ -3,3 +3,4 @@ test.dir=test
 build.dir=build
 test.class=com.google.inject.lifecycle.LifecycleTest
 module=com.google.inject.lifecycle
+fragment=true
diff --git a/servlet/build.properties b/servlet/build.properties
index da9d934b..36c3df5f 100644
--- a/servlet/build.properties
+++ b/servlet/build.properties
@@ -4,3 +4,4 @@ test.dir=test
 build.dir=build
 test.class=com.google.inject.servlet.AllTests
 module=com.google.inject.servlet
+fragment=true
diff --git a/spring/build.properties b/spring/build.properties
index a0789d2f..0538abf2 100644
--- a/spring/build.properties
+++ b/spring/build.properties
@@ -3,3 +3,4 @@ test.dir=test
 build.dir=build
 test.class=com.google.inject.spring.SpringTest
 module=com.google.inject.spring
+fragment=true
diff --git a/src/com/google/inject/internal/BytecodeGen.java b/src/com/google/inject/internal/BytecodeGen.java
index da008d43..2ec27698 100644
--- a/src/com/google/inject/internal/BytecodeGen.java
+++ b/src/com/google/inject/internal/BytecodeGen.java
@@ -16,7 +16,6 @@
 
 package com.google.inject.internal;
 
-import static com.google.inject.internal.Preconditions.checkNotNull;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
@@ -55,19 +54,24 @@ import java.util.logging.Logger;
  * @author mcculls@gmail.com (Stuart McCulloch)
  * @author jessewilson@google.com (Jesse Wilson)
  */
-final class BytecodeGen {
+public final class BytecodeGen {
 
-  private static final Logger logger = Logger.getLogger(BytecodeGen.class.getName());
+  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());
+
+  // initialization-on-demand...
+  private static class SystemBridgeHolder {
+    static final BridgeClassLoader SYSTEM_BRIDGE = new BridgeClassLoader();
+  }
 
   /** ie. "com.google.inject.internal" */
-  private static final String GUICE_INTERNAL_PACKAGE
+  static final String GUICE_INTERNAL_PACKAGE
       = BytecodeGen.class.getName().replaceFirst("\\.internal\\..*$", ".internal");
 
   /*if[AOP]*/
   /** either "net.sf.cglib", or "com.google.inject.internal.cglib" */
-  private static final String CGLIB_PACKAGE
+  static final String CGLIB_PACKAGE
       = net.sf.cglib.proxy.Enhancer.class.getName().replaceFirst("\\.cglib\\..*$", ".cglib");
 
   static final net.sf.cglib.core.NamingPolicy NAMING_POLICY
@@ -82,47 +86,39 @@ final class BytecodeGen {
   end[NO_AOP]*/
 
   /** Use "-Dguice.custom.loader=false" to disable custom classloading. */
-  static final boolean HOOK_ENABLED
-      = "true".equals(System.getProperty("guice.custom.loader", "true"));
+  private static final boolean CUSTOM_LOADER_ENABLED
+      = Boolean.parseBoolean(System.getProperty("guice.custom.loader", "true"));
 
   /**
    * 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
-      = new MapMaker().weakKeys().weakValues().makeComputingMap(
+  private static final Map<ClassLoader, ClassLoader> CLASS_LOADER_CACHE;
+
+  static {
+    if (CUSTOM_LOADER_ENABLED) {
+      CLASS_LOADER_CACHE = new MapMaker().weakKeys().weakValues().makeComputingMap(
           new Function<ClassLoader, ClassLoader>() {
-    public ClassLoader apply(final @Nullable ClassLoader typeClassLoader) {
-      logger.fine("Creating a bridge ClassLoader for " + typeClassLoader);
-      return AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
-        public ClassLoader run() {
-          return new BridgeClassLoader(typeClassLoader);
-        }
-      });
+            public ClassLoader apply(final @Nullable ClassLoader typeClassLoader) {
+              logger.fine("Creating a bridge ClassLoader for " + typeClassLoader);
+              return AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
+                public ClassLoader run() {
+                  return new BridgeClassLoader(typeClassLoader);
+                }
+              });
+            }
+          });
+    } else {
+      CLASS_LOADER_CACHE = ImmutableMap.of();
     }
-  });
-
-  /**
-   * For class loaders, {@code null}, is always an alias to the
-   * {@link ClassLoader#getSystemClassLoader() system class loader}. This method
-   * will not return null.
-   */
-  private static ClassLoader canonicalize(ClassLoader classLoader) {
-    return classLoader != null
-        ? classLoader
-        : checkNotNull(getSystemClassLoaderOrNull(), "Couldn't get a ClassLoader");
   }
 
   /**
-   * Returns the system classloader, or {@code null} if we don't have
-   * permission.
+   * Attempts to canonicalize null references to the system class loader.
+   * May return null if for some reason the system loader is unavailable.
    */
-  private static ClassLoader getSystemClassLoaderOrNull() {
-    try {
-      return ClassLoader.getSystemClassLoader();
-    } catch (SecurityException e) {
-      return null;
-    }
+  private static ClassLoader canonicalize(ClassLoader classLoader) {
+    return classLoader != null ? classLoader : SystemBridgeHolder.SYSTEM_BRIDGE.getParent();
   }
 
   /**
@@ -133,23 +129,30 @@ final class BytecodeGen {
   }
 
   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: do nothing!
+    if (!CUSTOM_LOADER_ENABLED) {
       return delegate;
     }
 
-    // Don't bother bridging existing bridge classloaders
-    if (delegate instanceof BridgeClassLoader) {
+    delegate = canonicalize(delegate);
+
+    // no need for a bridge if using same class loader, or it's already a bridge
+    if (delegate == GUICE_CLASS_LOADER || delegate instanceof BridgeClassLoader) {
       return delegate;
     }
 
-    if (HOOK_ENABLED && Visibility.forType(type) == Visibility.PUBLIC) {
-      return CLASS_LOADER_CACHE.get(delegate);
+    // don't try bridging private types as it won't work
+    if (Visibility.forType(type) == Visibility.PUBLIC) {
+      if (delegate != SystemBridgeHolder.SYSTEM_BRIDGE.getParent()) {
+        // delegate guaranteed to be non-null here
+        return CLASS_LOADER_CACHE.get(delegate);
+      }
+      // delegate may or may not be null here
+      return SystemBridgeHolder.SYSTEM_BRIDGE;
     }
 
-    return delegate;
+    return delegate; // last-resort: do nothing!
   }
 
   /*if[AOP]*/
@@ -193,6 +196,7 @@ final class BytecodeGen {
      * target class. These generated classes may be loaded by our bridge classloader.
      */
     PUBLIC {
+      @Override
       public Visibility and(Visibility that) {
         return that;
       }
@@ -205,6 +209,7 @@ final class BytecodeGen {
      * garbage collected.
      */
     SAME_PACKAGE {
+      @Override
       public Visibility and(Visibility that) {
         return this;
       }
@@ -242,26 +247,44 @@ final class BytecodeGen {
    */
   private static class BridgeClassLoader extends ClassLoader {
 
-    public BridgeClassLoader(ClassLoader usersClassLoader) {
+    BridgeClassLoader() {
+      // use system loader as parent
+    }
+
+    BridgeClassLoader(ClassLoader usersClassLoader) {
       super(usersClassLoader);
     }
 
     @Override protected Class<?> loadClass(String name, boolean resolve)
         throws ClassNotFoundException {
 
-      // delegate internal requests to Guice class space
+      if (name.startsWith("sun.reflect")) {
+        // these reflection classes must be loaded from bootstrap class loader
+        return SystemBridgeHolder.SYSTEM_BRIDGE.classicLoadClass(name, resolve);
+      }
+
       if (name.startsWith(GUICE_INTERNAL_PACKAGE) || name.startsWith(CGLIB_PACKAGE)) {
+        if (null == GUICE_CLASS_LOADER) {
+          // use special system bridge to load classes from bootstrap class loader
+          return SystemBridgeHolder.SYSTEM_BRIDGE.classicLoadClass(name, resolve);
+        }
         try {
           Class<?> clazz = GUICE_CLASS_LOADER.loadClass(name);
           if (resolve) {
             resolveClass(clazz);
           }
           return clazz;
-        } catch (Exception e) {
-          // fall back to classic delegation
+        } catch (Throwable e) {
+          // fall-back to classic delegation
         }
       }
 
+      return classicLoadClass(name, resolve);
+    }
+
+    // make the classic delegating loadClass method visible
+    Class<?> classicLoadClass(String name, boolean resolve)
+      throws ClassNotFoundException {
       return super.loadClass(name, resolve);
     }
   }
diff --git a/src/com/google/inject/internal/DefaultConstructionProxyFactory.java b/src/com/google/inject/internal/DefaultConstructionProxyFactory.java
index 6bcaeef6..505ffa80 100644
--- a/src/com/google/inject/internal/DefaultConstructionProxyFactory.java
+++ b/src/com/google/inject/internal/DefaultConstructionProxyFactory.java
@@ -46,13 +46,14 @@ final class DefaultConstructionProxyFactory<T> implements ConstructionProxyFacto
 
     // Use FastConstructor if the constructor is public.
     if (Modifier.isPublic(constructor.getModifiers())) {
+      Class<T> classToConstruct = constructor.getDeclaringClass();
       /*if[AOP]*/
-      return new ConstructionProxy<T>() {
-        Class<T> classToConstruct = constructor.getDeclaringClass();
+      try {
         final net.sf.cglib.reflect.FastConstructor fastConstructor
             = BytecodeGen.newFastClass(classToConstruct, Visibility.forMember(constructor))
                 .getConstructor(constructor);
 
+      return new ConstructionProxy<T>() {
         @SuppressWarnings("unchecked")
         public T newInstance(Object... arguments) throws InvocationTargetException {
           return (T) fastConstructor.newInstance(arguments);
@@ -68,7 +69,11 @@ final class DefaultConstructionProxyFactory<T> implements ConstructionProxyFacto
           return ImmutableMap.of();
         }
       };
+      } catch (net.sf.cglib.core.CodeGenerationException e) {/* fall-through */}
       /*end[AOP]*/
+      if (!Modifier.isPublic(classToConstruct.getModifiers())) {
+        constructor.setAccessible(true);
+      }
     } else {
       constructor.setAccessible(true);
     }
diff --git a/src/com/google/inject/internal/ProxyFactory.java b/src/com/google/inject/internal/ProxyFactory.java
index ccd4b7cf..42b634c8 100644
--- a/src/com/google/inject/internal/ProxyFactory.java
+++ b/src/com/google/inject/internal/ProxyFactory.java
@@ -17,6 +17,8 @@
 package com.google.inject.internal;
 
 import static com.google.inject.internal.BytecodeGen.newFastClass;
+
+import com.google.inject.ProvisionException;
 import com.google.inject.spi.InjectionPoint;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
@@ -153,10 +155,14 @@ final class ProxyFactory<T> implements ConstructionProxyFactory<T> {
 
     // Create the proxied class. We're careful to ensure that all enhancer state is not-specific
     // to this injector. Otherwise, the proxies for each injector will waste PermGen memory
+    try {
     Enhancer enhancer = BytecodeGen.newEnhancer(declaringClass, visibility);
     enhancer.setCallbackFilter(new IndicesCallbackFilter(declaringClass, methods));
     enhancer.setCallbackTypes(callbackTypes);
     return new ProxyConstructor<T>(enhancer, injectionPoint, callbacks, interceptors);
+    } catch (Throwable e) {
+      throw new ProvisionException("Unable to method intercept: " + declaringClass, e);
+    }
   }
 
   private static class MethodInterceptorsPair {
diff --git a/src/com/google/inject/internal/SingleMethodInjector.java b/src/com/google/inject/internal/SingleMethodInjector.java
index d0f4c950..daf4bc05 100644
--- a/src/com/google/inject/internal/SingleMethodInjector.java
+++ b/src/com/google/inject/internal/SingleMethodInjector.java
@@ -45,6 +45,7 @@ final class SingleMethodInjector implements SingleMemberInjector {
     int modifiers = method.getModifiers();
     if (!Modifier.isPrivate(modifiers) && !Modifier.isProtected(modifiers)) {
       /*if[AOP]*/
+      try {
       final net.sf.cglib.reflect.FastMethod fastMethod
           = BytecodeGen.newFastClass(method.getDeclaringClass(), Visibility.forMember(method))
               .getMethod(method);
@@ -55,10 +56,12 @@ final class SingleMethodInjector implements SingleMemberInjector {
           return fastMethod.invoke(target, parameters);
         }
       };
+      } catch (net.sf.cglib.core.CodeGenerationException e) {/* fall-through */}
       /*end[AOP]*/
     }
 
-    if (!Modifier.isPublic(modifiers)) {
+    if (!Modifier.isPublic(modifiers) ||
+        !Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
       method.setAccessible(true);
     }
 
diff --git a/struts2/plugin/build.properties b/struts2/plugin/build.properties
index f1d0ef72..c3598693 100644
--- a/struts2/plugin/build.properties
+++ b/struts2/plugin/build.properties
@@ -2,3 +2,4 @@ lib.dir=../lib
 src.dir=src
 build.dir=build
 module=com.google.inject.struts2
+fragment=true
diff --git a/test/com/google/inject/AllTests.java b/test/com/google/inject/AllTests.java
index 537bee6b..6c2929c1 100644
--- a/test/com/google/inject/AllTests.java
+++ b/test/com/google/inject/AllTests.java
@@ -40,7 +40,6 @@ import com.google.inject.spi.ToolStageInjectorTest;
 import com.google.inject.util.NoopOverrideTest;
 import com.google.inject.util.ProvidersTest;
 import com.google.inject.util.TypesTest;
-import com.googlecode.guice.BytecodeGenTest;
 import com.googlecode.guice.Jsr330Test;
 import com.googlecode.guice.GuiceTck;
 import java.util.Enumeration;
@@ -143,11 +142,11 @@ public class AllTests {
     suite.addTestSuite(IntegrationTest.class);
     suite.addTestSuite(MethodInterceptionTest.class);
     suite.addTestSuite(com.googlecode.guice.BytecodeGenTest.class);
-    suite.addTest(com.googlecode.guice.StrictContainerTestSuite.suite());
     /*end[AOP]*/
 
     // googlecode.guice
-    suite.addTestSuite(BytecodeGenTest.class);
+    suite.addTest(com.googlecode.guice.StrictContainerTestSuite.suite());
+    suite.addTestSuite(com.googlecode.guice.OSGiContainerTest.class);
     suite.addTestSuite(Jsr330Test.class);
 
     return removeSuppressedTests(suite, SUPPRESSED_TEST_NAMES);
diff --git a/test/com/googlecode/guice/BytecodeGenTest.java b/test/com/googlecode/guice/BytecodeGenTest.java
index 83987eae..882049ca 100644
--- a/test/com/googlecode/guice/BytecodeGenTest.java
+++ b/test/com/googlecode/guice/BytecodeGenTest.java
@@ -190,11 +190,9 @@ public class BytecodeGenTest extends TestCase {
       }
     }).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);
     } else {
-      assertNotSame(testProxy.getClass().getClassLoader(), ProxyTest.class.getClassLoader());
       assertNotSame(testProxy.getClass().getClassLoader(), systemClassLoader);
     }
   }
@@ -217,8 +215,19 @@ public class BytecodeGenTest extends TestCase {
      * this should be enough to queue the weak reference
      * unless something is holding onto it accidentally.
      */
+    String[] buf;
     System.gc();
+    buf = new String[8 * 1024 * 1024];
+    buf = null;
     System.gc();
+    buf = new String[8 * 1024 * 1024];
+    buf = null;
+    System.gc();
+    buf = new String[8 * 1024 * 1024];
+    buf = null;
+    System.gc();
+    buf = new String[8 * 1024 * 1024];
+    buf = null;
     System.gc();
 
     // This test could be somewhat flaky when the GC isn't working.
diff --git a/test/com/googlecode/guice/OSGiContainerTest.java b/test/com/googlecode/guice/OSGiContainerTest.java
new file mode 100644
index 00000000..01027ec2
--- /dev/null
+++ b/test/com/googlecode/guice/OSGiContainerTest.java
@@ -0,0 +1,141 @@
+/**
+ * Copyright (C) 2009 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.googlecode.guice;
+
+import java.io.BufferedOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.Iterator;
+import java.util.Properties;
+
+import javax.imageio.spi.ServiceRegistry;
+
+import junit.framework.TestCase;
+
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.launch.Framework;
+import org.osgi.framework.launch.FrameworkFactory;
+
+import aQute.bnd.main.bnd;
+
+import com.googlecode.guice.bundle.OSGiTestActivator;
+
+/**
+ * Run various tests inside one or more OSGi containers.
+ * 
+ * @author mcculls@gmail.com (Stuart McCulloch)
+ */
+public class OSGiContainerTest
+    extends TestCase {
+
+  // build properties passed from Ant
+  static final String VERSION = System.getProperty("version", "snapshot");
+  static final String BUILD_DIR = System.getProperty("build.dir", "build");
+
+  static final String BUILD_DIST_DIR = BUILD_DIR + "/dist";
+  static final String BUILD_TEST_DIR = BUILD_DIR + "/test";
+
+  static final String GUICE_JAR = BUILD_DIST_DIR + "/guice-" + VERSION + ".jar";
+
+  static final String AOPALLIANCE_JAR = System.getProperty("aopalliance.jar", "lib/aopalliance.jar");
+  static final String JAVAX_INJECT_JAR = System.getProperty("javax.inject.jar", "lib/javax.inject.jar");
+
+  // dynamically build test bundles
+  @Override protected void setUp()
+      throws Exception {
+
+    // verify properties
+    assertTrue(failMsg(), new File(BUILD_DIR).isDirectory());
+    assertTrue(failMsg(), new File(GUICE_JAR).isFile());
+
+    assertTrue(failMsg(), new File(AOPALLIANCE_JAR).isFile());
+    assertTrue(failMsg(), new File(JAVAX_INJECT_JAR).isFile());
+
+    Properties instructions = new Properties();
+
+    // aopalliance is an API bundle --> export the full API
+    instructions.setProperty("Export-Package", "org.aopalliance.*");
+    buildBundle("aopalliance", instructions, AOPALLIANCE_JAR);
+    instructions.clear();
+
+    // javax.inject is an API bundle --> export the full API
+    instructions.setProperty("Export-Package", "javax.inject.*");
+    buildBundle("javax.inject", instructions, JAVAX_INJECT_JAR);
+    instructions.clear();
+
+    // strict imports to make sure test bundle only has access to these packages
+    instructions.setProperty("Import-Package", "org.osgi.framework,org.aopalliance.intercept,"
+        + "com.google.inject(|.binder|.matcher|.name)");
+
+    // test bundle should only contain the local test classes, nothing else
+    instructions.setProperty("Bundle-Activator", OSGiTestActivator.class.getName());
+    instructions.setProperty("Private-Package", OSGiTestActivator.class.getPackage().getName());
+    buildBundle("osgitests", instructions, BUILD_TEST_DIR);
+    instructions.clear();
+  }
+
+  // build an OSGi bundle at runtime
+  private static void buildBundle(String name, Properties instructions, String classpath)
+      throws IOException {
+
+    // write BND instructions to temporary test directory
+    String bndFileName = BUILD_TEST_DIR + '/' + name + ".bnd";
+    OutputStream os = new BufferedOutputStream(new FileOutputStream(bndFileName));
+    instructions.store(os, "BND instructions");
+    os.close();
+
+    // assemble bundle, use -failok switch to avoid early exit
+    bnd.main(new String[]{"-failok", "build", "-classpath", classpath, bndFileName});
+  }
+  
+  private String failMsg() {
+    return "This test may fail if it is not run from ant, or if it is not run after ant has "
+         + "compiled & built jars. This is because the test is validating that the Guice jar "
+         + "is properly setup to load in an OSGi container";
+  }
+
+  //This test may fail if it is not run from ant, or if it is not run after ant has
+  //compiled & built jars. This is because the test is validating that the Guice jar
+  //is properly setup to load in an OSGi container
+  public void testGuiceWorksInOSGiContainer()
+      throws Throwable {
+
+    // ask framework to clear cache on startup
+    Properties properties = new Properties();
+    properties.setProperty("org.osgi.framework.storage", BUILD_TEST_DIR + "/bundle.cache");
+    properties.setProperty("org.osgi.framework.storage.clean", "onFirstInit");
+
+    // test each available OSGi framework in turn
+    Iterator<FrameworkFactory> f = ServiceRegistry.lookupProviders(FrameworkFactory.class);
+    while (f.hasNext()) {
+      Framework framework = f.next().newFramework(properties);
+
+      framework.start();
+      BundleContext systemContext = framework.getBundleContext();
+
+      // load all the necessary bundles and start the OSGi test bundle
+      systemContext.installBundle("reference:file:" + BUILD_TEST_DIR + "/aopalliance.jar");
+      systemContext.installBundle("reference:file:" + BUILD_TEST_DIR + "/javax.inject.jar");
+      systemContext.installBundle("reference:file:" + GUICE_JAR);
+      systemContext.installBundle("reference:file:" + BUILD_TEST_DIR + "/osgitests.jar").start();
+
+      framework.stop();
+    }
+  }
+}
diff --git a/test/com/googlecode/guice/bundle/OSGiTestActivator.java b/test/com/googlecode/guice/bundle/OSGiTestActivator.java
new file mode 100644
index 00000000..0527da69
--- /dev/null
+++ b/test/com/googlecode/guice/bundle/OSGiTestActivator.java
@@ -0,0 +1,513 @@
+/**
+ * Copyright (C) 2009 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.googlecode.guice.bundle;
+
+import static com.google.inject.name.Names.named;
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Random;
+
+import org.aopalliance.intercept.MethodInterceptor;
+import org.aopalliance.intercept.MethodInvocation;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleActivator;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.BundleException;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.matcher.AbstractMatcher;
+
+/**
+ * Test Guice from inside an OSGi bundle activator.
+ * 
+ * @author mcculls@gmail.com (Stuart McCulloch)
+ */
+@SuppressWarnings("unused") public class OSGiTestActivator
+    implements BundleActivator {
+
+  // varying visibilities to test our code-generation support
+
+  public static class _ {}
+
+  public interface A {}
+
+  protected interface B {}
+
+  interface C {}
+
+  private interface D {}
+
+  public static class AA
+      implements A {
+
+    public AA() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  protected static class AB
+      implements A {
+
+    public AB() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  static class AC
+      implements A {
+
+    public AC() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  private static class AD
+      implements A {
+
+    public AD() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  public static class BA
+      implements B {
+
+    protected BA() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  protected static class BB
+      implements B {
+
+    protected BB() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  static class BC
+      implements B {
+
+    protected BC() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  private static class BD
+      implements B {
+
+    protected BD() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  public static class CA
+      implements C {
+
+    CA() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  protected static class CB
+      implements C {
+
+    CB() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  static class CC
+      implements C {
+
+    CC() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  private static class CD
+      implements C {
+
+    CD() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  public static class DA
+      implements D {
+
+    @Inject private DA() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  protected static class DB
+      implements D {
+
+    @Inject private DB() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  static class DC
+      implements D {
+
+    @Inject private DC() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  private static class DD
+      implements D {
+
+    private DD() {}
+
+    @Inject public void setA(_ _) {}
+
+    @Inject protected void setB(_ _) {}
+
+    @Inject void setC(_ _) {}
+
+    @Inject private void setD(_ _) {}
+
+    @Inject public _ a;
+
+    @Inject protected _ b;
+
+    @Inject _ c;
+
+    @Inject private _ d;
+  }
+
+  enum Visibility {
+    PUBLIC, PROTECTED, PACKAGE_PRIVATE, PRIVATE
+  }
+
+  static final Class<?>[] TEST_CLAZZES = {A.class, B.class, C.class, D.class};
+
+  // registers all the class combinations
+  static class TestModule
+      extends AbstractModule {
+
+    final Bundle bundle;
+
+    TestModule(Bundle bundle) {
+      this.bundle = bundle;
+    }
+
+    @Override @SuppressWarnings("unchecked") protected void configure() {
+      for (Class<?> api : TEST_CLAZZES) {
+        for (Visibility visibility : Visibility.values()) {
+          try {
+
+            // this registers: A + PUBLIC -> AA, A + PROTECTED -> AB, etc...
+            String suffix = TEST_CLAZZES[visibility.ordinal()].getSimpleName();
+            Class imp = bundle.loadClass(api.getName() + suffix);
+            bind(api).annotatedWith(named(visibility.name())).to(imp);
+
+          } catch (ClassNotFoundException e) {
+            throw new RuntimeException("Unable to load test class", e);
+          }
+        }
+      }
+    }
+  }
+
+/*if[AOP]*/
+  // applies method-interception to classes with enough visibility
+  static class InterceptorModule
+      extends AbstractModule {
+    @Override protected void configure() {
+      bindInterceptor(new AbstractMatcher<Class<?>>() {
+        public boolean matches(Class<?> clazz) {
+          try {
+
+            // the class and constructor must be visible
+            int clazzModifiers = clazz.getModifiers();
+            int ctorModifiers = clazz.getConstructor().getModifiers();
+            return (clazzModifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) != 0
+                && (ctorModifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) != 0;
+
+          } catch (NoSuchMethodException e) {
+            return false;
+          }
+        }
+      }, new AbstractMatcher<Method>() {
+        public boolean matches(Method method) {
+
+          // the intercepted method must also be visible
+          int methodModifiers = method.getModifiers();
+          return (methodModifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) != 0;
+
+        }
+      }, new MethodInterceptor() {
+        public Object invoke(MethodInvocation mi)
+            throws Throwable {
+
+          return mi.proceed();
+        }
+      });
+    }
+  }
+/*end[AOP]*/
+
+  // called from OSGi when bundle starts
+  public void start(BundleContext context)
+      throws BundleException {
+
+    final Bundle bundle = context.getBundle();
+
+    Injector injector = Guice.createInjector(new TestModule(bundle));
+/*if[AOP]*/
+    Injector aopInjector = Guice.createInjector(new TestModule(bundle), new InterceptorModule());
+/*end[AOP]*/
+
+    // test code-generation support
+    for (Class<?> api : TEST_CLAZZES) {
+      for (Visibility vis : Visibility.values()) {
+        injector.getInstance(Key.get(api, named(vis.name())));
+/*if[AOP]*/
+        aopInjector.getInstance(Key.get(api, named(vis.name())));
+/*end[AOP]*/
+      }
+    }
+
+    // test injection of system class (issue 343)
+    injector.getInstance(Random.class);
+/*if[AOP]*/
+    aopInjector.getInstance(Random.class);
+/*end[AOP]*/
+  }
+
+  // called from OSGi when bundle stops
+  public void stop(BundleContext context) {}
+}