From c0ae78f3ec8f9baec6723b4f5ce3761144d7067e Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 22 Oct 2013 17:13:20 +0100 Subject: [PATCH] Isolate Aether in a separate class loader Prior to this commit, the Aether-based GrapeEngine was loaded in the same class loader as the rest of Boot. This led to Aether's and its dependencies' types polluting the application's class path. Most notably, this caused problems with logging as the logging framework could be permaturely initialized. This commit isolates AetherGrapeEngine, Aether and its dependencies into a separate class loader. This is done by customizing the packaging of the CLI's jar file with the internal directory housing all of the types that will be loaded by the separate class loader. --- pom.xml | 1 + spring-boot-cli-grape/pom.xml | 175 ++++++++++++++++++ .../boot/cli/compiler/AetherGrapeEngine.java | 47 ++--- .../DependencyResolutionFailedException.java | 0 .../org/springframework/boot/git.properties | 13 ++ spring-boot-cli/pom.xml | 84 ++------- .../{descriptor.xml => bin-package.xml} | 16 +- .../src/main/assembly/repackage-jar.xml | 25 +++ .../cli/command/tester/AbstractTester.java | 42 ----- .../boot/cli/compiler/GroovyCompiler.java | 24 ++- .../org/springframework/boot/git.properties | 13 ++ spring-boot-full-build/pom.xml | 1 + spring-boot-parent/pom.xml | 48 +++-- .../spring-boot-maven-plugin/pom.xml | 22 ++- 14 files changed, 344 insertions(+), 167 deletions(-) create mode 100644 spring-boot-cli-grape/pom.xml rename {spring-boot-cli => spring-boot-cli-grape}/src/main/java/org/springframework/boot/cli/compiler/AetherGrapeEngine.java (88%) rename {spring-boot-cli => spring-boot-cli-grape}/src/main/java/org/springframework/boot/cli/compiler/DependencyResolutionFailedException.java (100%) create mode 100644 spring-boot-cli-grape/src/main/resources/org/springframework/boot/git.properties rename spring-boot-cli/src/main/assembly/{descriptor.xml => bin-package.xml} (74%) create mode 100644 spring-boot-cli/src/main/assembly/repackage-jar.xml delete mode 100644 spring-boot-cli/src/main/java/org/springframework/boot/cli/command/tester/AbstractTester.java create mode 100644 spring-boot-cli/src/main/resources/org/springframework/boot/git.properties diff --git a/pom.xml b/pom.xml index 5be17f66d8e..d0d1d5faaf3 100644 --- a/pom.xml +++ b/pom.xml @@ -37,6 +37,7 @@ spring-boot-actuator spring-boot-starters spring-boot-cli + spring-boot-cli-grape spring-boot-integration-tests diff --git a/spring-boot-cli-grape/pom.xml b/spring-boot-cli-grape/pom.xml new file mode 100644 index 00000000000..39724177a01 --- /dev/null +++ b/spring-boot-cli-grape/pom.xml @@ -0,0 +1,175 @@ + + + 4.0.0 + + org.springframework.boot + spring-boot-parent + 0.5.0.BUILD-SNAPSHOT + ../spring-boot-parent + + spring-boot-cli-grape + jar + + ${basedir}/.. + org.springframework.boot.cli.SpringCli + + + + + org.codehaus.groovy + groovy + provided + + + + commons-logging + commons-logging + 1.1.3 + + + org.apache.maven + maven-aether-provider + + + org.eclipse.sisu.plexus + org.eclipse.sisu + + + + + org.eclipse.aether + aether-api + + + org.eclipse.aether + aether-connector-basic + + + org.eclipse.aether + aether-impl + + + org.eclipse.aether + aether-spi + + + org.eclipse.aether + aether-transport-file + + + org.eclipse.aether + aether-transport-http + + + jcl-over-slf4j + org.slf4j + + + + + org.eclipse.aether + aether-util + + + + junit + junit + test + + + + + + maven-surefire-plugin + + + org.springframework:spring-core + org.springframework:spring-beans + org.springframework:spring-aop + org.springframework:spring-tx + org.springframework:spring-expression + org.springframework:spring-context + org.springframework:spring-test + org.springframework.retry:spring-retry + org.springframework.integration:spring-integration-core + org.springframework.integration:spring-integration-dsl-groovy-core + + + + + maven-shade-plugin + + + + *:* + + META-INF/*.SF + META-INF/*.DSA + META-INF/*.RSA + + + + + + + package + + shade + + + + + + ${start-class} + + + false + + + + + + + + + + org.eclipse.m2e + lifecycle-mapping + 1.0.0 + + + + + + org.apache.maven.plugins + maven-antrun-plugin + [1.7,) + + run + + + + + + + + + + + + + + + + objectstyle + ObjectStyle.org Repository + http://objectstyle.org/maven2/ + + false + + + + diff --git a/spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/AetherGrapeEngine.java b/spring-boot-cli-grape/src/main/java/org/springframework/boot/cli/compiler/AetherGrapeEngine.java similarity index 88% rename from spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/AetherGrapeEngine.java rename to spring-boot-cli-grape/src/main/java/org/springframework/boot/cli/compiler/AetherGrapeEngine.java index 26e60ab346d..e3aa293c3e1 100644 --- a/spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/AetherGrapeEngine.java +++ b/spring-boot-cli-grape/src/main/java/org/springframework/boot/cli/compiler/AetherGrapeEngine.java @@ -17,6 +17,7 @@ package org.springframework.boot.cli.compiler; import groovy.grape.GrapeEngine; +import groovy.lang.GroovyClassLoader; import java.io.File; import java.net.MalformedURLException; @@ -52,8 +53,6 @@ import org.eclipse.aether.resolution.DependencyRequest; import org.eclipse.aether.resolution.DependencyResult; import org.eclipse.aether.spi.connector.RepositoryConnectorFactory; import org.eclipse.aether.spi.connector.transport.TransporterFactory; -import org.eclipse.aether.spi.log.Logger; -import org.eclipse.aether.spi.log.LoggerFactory; import org.eclipse.aether.transfer.AbstractTransferListener; import org.eclipse.aether.transfer.TransferCancelledException; import org.eclipse.aether.transfer.TransferEvent; @@ -78,13 +77,13 @@ public class AetherGrapeEngine implements GrapeEngine { private static final String DEPENDENCY_VERSION = "version"; - private final ProgressReporter progressReporter = new ProgressReporter(); + private final Artifact parentArtifact; - private final ArtifactCoordinatesResolver artifactCoordinatesResolver; + private final ProgressReporter progressReporter = new ProgressReporter(); private final ArtifactDescriptorReader artifactDescriptorReader; - private final ExtendedGroovyClassLoader defaultClassLoader; + private final GroovyClassLoader defaultClassLoader; private final RepositorySystemSession repositorySystemSession; @@ -92,14 +91,14 @@ public class AetherGrapeEngine implements GrapeEngine { private final List repositories; - public AetherGrapeEngine(ExtendedGroovyClassLoader classLoader, - ArtifactCoordinatesResolver artifactCoordinatesResolver) { + public AetherGrapeEngine(GroovyClassLoader classLoader, String parentGroupId, + String parentArtifactId, String parentVersion) { this.defaultClassLoader = classLoader; - this.artifactCoordinatesResolver = artifactCoordinatesResolver; + this.parentArtifact = new DefaultArtifact(parentGroupId, parentArtifactId, "pom", + parentVersion); DefaultServiceLocator mavenServiceLocator = MavenRepositorySystemUtils .newServiceLocator(); - mavenServiceLocator.setService(LoggerFactory.class, NopLoggerFactory.class); mavenServiceLocator.addService(RepositorySystem.class, DefaultRepositorySystem.class); @@ -166,8 +165,7 @@ public class AetherGrapeEngine implements GrapeEngine { try { List files = resolve(dependencies); - ExtendedGroovyClassLoader classLoader = (ExtendedGroovyClassLoader) args - .get("classLoader"); + GroovyClassLoader classLoader = (GroovyClassLoader) args.get("classLoader"); if (classLoader == null) { classLoader = this.defaultClassLoader; } @@ -233,10 +231,7 @@ public class AetherGrapeEngine implements GrapeEngine { private List getManagedDependencies() { ArtifactDescriptorRequest parentRequest = new ArtifactDescriptorRequest(); - Artifact parentArtifact = new DefaultArtifact("org.springframework.boot", - "spring-boot-starter-parent", "pom", - this.artifactCoordinatesResolver.getVersion("spring-boot")); - parentRequest.setArtifact(parentArtifact); + parentRequest.setArtifact(this.parentArtifact); try { ArtifactDescriptorResult artifactDescriptorResult = this.artifactDescriptorReader @@ -250,41 +245,33 @@ public class AetherGrapeEngine implements GrapeEngine { @Override public Map>> enumerateGrapes() { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("Grape enumeration is not supported"); } @Override public URI[] resolve(Map args, Map... dependencies) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("Resolving to URIs is not supported"); } @Override public URI[] resolve(Map args, List depsInfo, Map... dependencies) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("Resolving to URIs is not supported"); } @Override public Map[] listDependencies(ClassLoader classLoader) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("Listing dependencies is not supported"); } @Override public void addResolver(Map args) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("Adding a resolver is not supported"); } @Override public Object grab(String endorsedModule) { - throw new UnsupportedOperationException("Auto-generated method stub"); - } - - private static final class NopLoggerFactory implements LoggerFactory { - - @Override - public Logger getLogger(String name) { - // TODO Something more robust; I'm surprised this doesn't NPE - return null; - } + throw new UnsupportedOperationException( + "Grabbing an endorsed module is not supported"); } private static final class ProgressReporter { diff --git a/spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/DependencyResolutionFailedException.java b/spring-boot-cli-grape/src/main/java/org/springframework/boot/cli/compiler/DependencyResolutionFailedException.java similarity index 100% rename from spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/DependencyResolutionFailedException.java rename to spring-boot-cli-grape/src/main/java/org/springframework/boot/cli/compiler/DependencyResolutionFailedException.java diff --git a/spring-boot-cli-grape/src/main/resources/org/springframework/boot/git.properties b/spring-boot-cli-grape/src/main/resources/org/springframework/boot/git.properties new file mode 100644 index 00000000000..98537901ec0 --- /dev/null +++ b/spring-boot-cli-grape/src/main/resources/org/springframework/boot/git.properties @@ -0,0 +1,13 @@ +#Generated by Git-Commit-Id-Plugin +#Tue Oct 22 10:25:03 BST 2013 +git.commit.id.abbrev=040321b +git.commit.user.email=awilkinson@gopivotal.com +git.commit.message.full=Isolate Aether in a separate class loader\n\nPrior to this commit, the Aether-based GrapeEngine was loaded in the\nsame class loader as the rest of Boot. This led to Aether's and its\ndependencies' types polluting the application's class path. Most\nnotably, this caused problems with logging as the logging framework\ncould be permaturely initialized.\n\nThis commit isolates AetherGrapeEngine, Aether and its dependencies\ninto a separate class loader. This is done by customizing the\npackaging of the CLI's jar file with the internal directory housing\nall of the types that will be loaded by the separate class loader.\n +git.commit.id=040321bf153db007290786623d45903cee27fa88 +git.commit.message.short=Isolate Aether in a separate class loader +git.commit.user.name=Andy Wilkinson +git.build.user.name=Andy Wilkinson +git.build.user.email=awilkinson@gopivotal.com +git.branch=aether-grab +git.commit.time=2013-10-21T16\:24\:40+0100 +git.build.time=2013-10-22T10\:25\:03+0100 diff --git a/spring-boot-cli/pom.xml b/spring-boot-cli/pom.xml index 64a6fc71dc4..baef3529935 100644 --- a/spring-boot-cli/pom.xml +++ b/spring-boot-cli/pom.xml @@ -36,88 +36,26 @@ - - commons-logging - commons-logging - 1.1.3 - net.sf.jopt-simple jopt-simple - - org.apache.maven - maven-aether-provider - - - org.eclipse.sisu.plexus - org.eclipse.sisu - - - - - org.apache.maven - maven-settings-builder - ${maven.version} - org.codehaus.groovy groovy - - org.eclipse.aether - aether-api - - - org.eclipse.aether - aether-connector-basic - ${aether.version} - - - org.eclipse.aether - aether-impl - - - org.eclipse.aether - aether-spi - ${aether.version} - - - org.eclipse.aether - aether-transport-file - ${aether.version} - - - org.eclipse.aether - aether-transport-http - ${aether.version} - - - jcl-over-slf4j - org.slf4j - - - - - org.eclipse.aether - aether-util - org.codehaus.groovy groovy-templates true + - junit - junit - test - - - - org.springframework.integration - spring-integration-dsl-groovy-core + ${project.groupId} + spring-boot-cli-grape + ${project.version} provided @@ -138,6 +76,11 @@ + + junit + junit + test + org.javassist javassist @@ -202,7 +145,6 @@ implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer" /> - ${start-class} false @@ -215,8 +157,14 @@ false - src/main/assembly/descriptor.xml + src/main/assembly/repackage-jar.xml + src/main/assembly/bin-package.xml + + + ${start-class} + + diff --git a/spring-boot-cli/src/main/assembly/descriptor.xml b/spring-boot-cli/src/main/assembly/bin-package.xml similarity index 74% rename from spring-boot-cli/src/main/assembly/descriptor.xml rename to spring-boot-cli/src/main/assembly/bin-package.xml index beac4ee7d01..9d36cb33bc8 100644 --- a/spring-boot-cli/src/main/assembly/descriptor.xml +++ b/spring-boot-cli/src/main/assembly/bin-package.xml @@ -23,13 +23,11 @@ 755 - - - - org.springframework.boot:spring-boot-cli:jar:* - - lib - 755 - - + + + ${project.build.directory}/${project.artifactId}-${project.version}-repackaged.jar + /lib + ${project.build.finalName}.jar + + diff --git a/spring-boot-cli/src/main/assembly/repackage-jar.xml b/spring-boot-cli/src/main/assembly/repackage-jar.xml new file mode 100644 index 00000000000..4a809ff3ab2 --- /dev/null +++ b/spring-boot-cli/src/main/assembly/repackage-jar.xml @@ -0,0 +1,25 @@ + + repackaged + + jar + + false + + + + org.springframework.boot:spring-boot-cli:jar:* + + true + 755 + + + + org.springframework.boot:spring-boot-cli-grape:jar:* + + internal + 755 + provided + true + + + diff --git a/spring-boot-cli/src/main/java/org/springframework/boot/cli/command/tester/AbstractTester.java b/spring-boot-cli/src/main/java/org/springframework/boot/cli/command/tester/AbstractTester.java deleted file mode 100644 index ec7f19c2a80..00000000000 --- a/spring-boot-cli/src/main/java/org/springframework/boot/cli/command/tester/AbstractTester.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2012-2013 the original author or authors. - * - * 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 org.springframework.boot.cli.command.tester; - -import java.io.FileNotFoundException; -import java.util.List; -import java.util.Set; - -/** - * Abstract base class for tester implementations. - * - * @author Greg Turnquist - */ -public abstract class AbstractTester { - - public TestResults findAndTest(List> compiled) throws FileNotFoundException { - Set> testable = findTestableClasses(compiled); - if (testable.size() == 0) { - return TestResults.NONE; - } - return test(testable.toArray(new Class[] {})); - } - - protected abstract Set> findTestableClasses(List> compiled); - - protected abstract TestResults test(Class[] testable); - -} diff --git a/spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/GroovyCompiler.java b/spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/GroovyCompiler.java index 24e22b84cc5..2df6ac6fdc2 100644 --- a/spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/GroovyCompiler.java +++ b/spring-boot-cli/src/main/java/org/springframework/boot/cli/compiler/GroovyCompiler.java @@ -16,13 +16,17 @@ package org.springframework.boot.cli.compiler; +import groovy.grape.GrapeEngine; import groovy.lang.Grab; import groovy.lang.GroovyClassLoader; import groovy.lang.GroovyClassLoader.ClassCollector; import java.io.File; import java.io.IOException; +import java.lang.reflect.Constructor; import java.lang.reflect.Field; +import java.net.URL; +import java.net.URLClassLoader; import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedList; @@ -70,6 +74,9 @@ import org.codehaus.groovy.transform.ASTTransformationVisitor; */ public class GroovyCompiler { + private static final ClassLoader AETHER_CLASS_LOADER = new URLClassLoader( + new URL[] { GroovyCompiler.class.getResource("/internal/") }); + private GroovyCompilerConfiguration configuration; private ExtendedGroovyClassLoader loader; @@ -84,6 +91,7 @@ public class GroovyCompiler { * Create a new {@link GroovyCompiler} instance. * @param configuration the compiler configuration */ + @SuppressWarnings("unchecked") public GroovyCompiler(final GroovyCompilerConfiguration configuration) { this.configuration = configuration; CompilerConfiguration compilerConfiguration = new CompilerConfiguration(); @@ -95,8 +103,20 @@ public class GroovyCompiler { this.artifactCoordinatesResolver = new PropertiesArtifactCoordinatesResolver( this.loader); - new GrapeEngineInstaller(new AetherGrapeEngine(this.loader, - this.artifactCoordinatesResolver)).install(); + try { + Class grapeEngineClass = (Class) AETHER_CLASS_LOADER + .loadClass("org.springframework.boot.cli.compiler.AetherGrapeEngine"); + Constructor constructor = grapeEngineClass.getConstructor( + GroovyClassLoader.class, String.class, String.class, String.class); + GrapeEngine grapeEngine = constructor.newInstance(this.loader, + "org.springframework.boot", "spring-boot-starter-parent", + this.artifactCoordinatesResolver.getVersion("spring-boot")); + + new GrapeEngineInstaller(grapeEngine).install(); + } + catch (Exception ex) { + throw new IllegalStateException("Failed to install custom GrapeEngine", ex); + } compilerConfiguration .addCompilationCustomizers(new CompilerAutoConfigureCustomizer()); diff --git a/spring-boot-cli/src/main/resources/org/springframework/boot/git.properties b/spring-boot-cli/src/main/resources/org/springframework/boot/git.properties new file mode 100644 index 00000000000..2e6dcd63164 --- /dev/null +++ b/spring-boot-cli/src/main/resources/org/springframework/boot/git.properties @@ -0,0 +1,13 @@ +#Generated by Git-Commit-Id-Plugin +#Tue Oct 22 10:25:04 BST 2013 +git.commit.id.abbrev=040321b +git.commit.user.email=awilkinson@gopivotal.com +git.commit.message.full=Isolate Aether in a separate class loader\n\nPrior to this commit, the Aether-based GrapeEngine was loaded in the\nsame class loader as the rest of Boot. This led to Aether's and its\ndependencies' types polluting the application's class path. Most\nnotably, this caused problems with logging as the logging framework\ncould be permaturely initialized.\n\nThis commit isolates AetherGrapeEngine, Aether and its dependencies\ninto a separate class loader. This is done by customizing the\npackaging of the CLI's jar file with the internal directory housing\nall of the types that will be loaded by the separate class loader.\n +git.commit.id=040321bf153db007290786623d45903cee27fa88 +git.commit.message.short=Isolate Aether in a separate class loader +git.commit.user.name=Andy Wilkinson +git.build.user.name=Andy Wilkinson +git.build.user.email=awilkinson@gopivotal.com +git.branch=aether-grab +git.commit.time=2013-10-21T16\:24\:40+0100 +git.build.time=2013-10-22T10\:25\:04+0100 diff --git a/spring-boot-full-build/pom.xml b/spring-boot-full-build/pom.xml index e40acbb0a98..984e4e58923 100644 --- a/spring-boot-full-build/pom.xml +++ b/spring-boot-full-build/pom.xml @@ -16,6 +16,7 @@ ../spring-boot-actuator ../spring-boot-starters ../spring-boot-cli + ../spring-boot-cli-grape ../spring-boot-samples ../spring-boot-integration-tests ../spring-boot-javadoc diff --git a/spring-boot-parent/pom.xml b/spring-boot-parent/pom.xml index 972b22c5e9c..46b901f5e3f 100644 --- a/spring-boot-parent/pom.xml +++ b/spring-boot-parent/pom.xml @@ -109,20 +109,40 @@ 3.0.10 - org.eclipse.aether - aether-api - ${aether.version} - - - org.eclipse.aether - aether-impl - ${aether.version} - - - org.eclipse.aether - aether-util - ${aether.version} - + org.eclipse.aether + aether-api + ${aether.version} + + + org.eclipse.aether + aether-connector-basic + ${aether.version} + + + org.eclipse.aether + aether-impl + ${aether.version} + + + org.eclipse.aether + aether-spi + ${aether.version} + + + org.eclipse.aether + aether-transport-file + ${aether.version} + + + org.eclipse.aether + aether-transport-http + ${aether.version} + + + org.eclipse.aether + aether-util + ${aether.version} + org.gradle gradle-core diff --git a/spring-boot-tools/spring-boot-maven-plugin/pom.xml b/spring-boot-tools/spring-boot-maven-plugin/pom.xml index 7fee13139c3..84adf954c99 100644 --- a/spring-boot-tools/spring-boot-maven-plugin/pom.xml +++ b/spring-boot-tools/spring-boot-maven-plugin/pom.xml @@ -30,6 +30,12 @@ org.apache.maven maven-core + + + asm + asm + + org.apache.maven @@ -72,12 +78,24 @@ true - asm asm + asm - asm-commons asm + asm-analysis + + + asm + asm-commons + + + asm + asm-tree + + + asm + asm-util