Skip to content

Commit

Permalink
Patches to let Guice be more OSGi (and classloader) friendly. Solves …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
sberlin committed May 9, 2010
1 parent 34d2f00 commit f7ac6ea
Show file tree
Hide file tree
Showing 24 changed files with 787 additions and 69 deletions.
2 changes: 1 addition & 1 deletion build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
8 changes: 5 additions & 3 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down Expand Up @@ -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>
Expand Down
21 changes: 14 additions & 7 deletions common.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand All @@ -25,26 +25,33 @@
<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/"/>
<property name="Bundle-Copyright" value="Copyright (C) 2006 Google Inc."/>
<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}"/>
Expand Down
1 change: 1 addition & 0 deletions extensions/assistedinject/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ test.dir=test
build.dir=build
test.class=com.google.inject.assistedinject.FactoryProviderTest
module=com.google.inject.assistedinject
fragment=true
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
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;
Expand Down Expand Up @@ -365,7 +366,7 @@ public Object[] gatherArgsForConstructor(

@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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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;
Expand Down Expand Up @@ -214,7 +215,7 @@ public String toString() {
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));
}

Expand Down
1 change: 1 addition & 0 deletions extensions/grapher/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ test.dir=test
build.dir=build
test.class=com.google.inject.grapher.AllTests
module=com.google.inject.grapher
fragment=true
1 change: 1 addition & 0 deletions extensions/multibindings/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ test.dir=test
build.dir=build
test.class=com.google.inject.multibindings.AllTests
module=com.google.inject.multibindings
fragment=true
1 change: 1 addition & 0 deletions extensions/throwingproviders/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ test.dir=test
build.dir=build
test.class=com.google.inject.throwingproviders.ThrowingProviderBinderTest
module=com.google.inject.throwingproviders
fragment=true
Binary file removed lib/build/bnd-0.0.305.jar
Binary file not shown.
Binary file added lib/build/bnd-0.0.384.jar
Binary file not shown.
Binary file added lib/build/felix-2.0.5.jar
Binary file not shown.
1 change: 1 addition & 0 deletions lifecycle/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ test.dir=test
build.dir=build
test.class=com.google.inject.lifecycle.LifecycleTest
module=com.google.inject.lifecycle
fragment=true
1 change: 1 addition & 0 deletions servlet/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ test.dir=test
build.dir=build
test.class=com.google.inject.servlet.AllTests
module=com.google.inject.servlet
fragment=true
1 change: 1 addition & 0 deletions spring/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ test.dir=test
build.dir=build
test.class=com.google.inject.spring.SpringTest
module=com.google.inject.spring
fragment=true
119 changes: 71 additions & 48 deletions src/com/google/inject/internal/BytecodeGen.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -55,19 +54,24 @@
* @author [email protected] (Stuart McCulloch)
* @author [email protected] (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
Expand All @@ -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();
}

/**
Expand All @@ -133,23 +129,30 @@ public static ClassLoader getClassLoader(Class<?> type) {
}

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]*/
Expand Down Expand Up @@ -193,6 +196,7 @@ public enum Visibility {
* target class. These generated classes may be loaded by our bridge classloader.
*/
PUBLIC {
@Override
public Visibility and(Visibility that) {
return that;
}
Expand All @@ -205,6 +209,7 @@ public Visibility and(Visibility that) {
* garbage collected.
*/
SAME_PACKAGE {
@Override
public Visibility and(Visibility that) {
return this;
}
Expand Down Expand Up @@ -242,26 +247,44 @@ public static Visibility forType(Class<?> type) {
*/
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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,14 @@ public ConstructionProxy<T> create() {

// 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);
Expand All @@ -68,7 +69,11 @@ public Constructor<T> getConstructor() {
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);
}
Expand Down
Loading

0 comments on commit f7ac6ea

Please sign in to comment.