From c1edb3b5bd90a5548cf6db19f9bcf7383e4898fb Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Thu, 27 Apr 2017 11:21:27 -0700 Subject: [PATCH] Enforce limit on classes loaded by Spel compiled expression loader Until this change a single classloader was used to load all compiled SpEL expressions. This meant in a context where an expression was repeatedly flipping between compiled and interpreted mode (which can happen if in MIXED mode compilation and changing the context around the evaluation) the classloader would continually load a new compiled version but not orphan the old compiled version. This eventually uses up all the memory as the number of classes is ever increasing. With this change classloaders are used to load 100 compiled expressions. The 101st will be loaded by a new one. Orphaning the old classloader means if an expression is ever recompiled there is more likely to be no anchored references left to the older compiled form and it can be GC'd. In the MIXED situation above it should help alleviate the problem of older classes never being candidates for GC. Issue: SPR-15460 --- .../spel/standard/SpelCompiler.java | 37 +++++++++++++++---- .../spel/SpelCompilationCoverageTests.java | 21 +++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelCompiler.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelCompiler.java index 452c33b934..9d29944c34 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelCompiler.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelCompiler.java @@ -70,24 +70,23 @@ public class SpelCompiler implements Opcodes { private static final Log logger = LogFactory.getLog(SpelCompiler.class); + private final static int CLASSES_DEFINED_LIMIT = 100; + // A compiler is created for each classloader, it manages a child class loader of that // classloader and the child is used to load the compiled expressions. private static final Map compilers = new ConcurrentReferenceHashMap<>(); - // The child ClassLoader used to load the compiled expression classes - private final ChildClassLoader ccl; + private ChildClassLoader ccl; // Counter suffix for generated classes within this SpelCompiler instance private final AtomicInteger suffixId = new AtomicInteger(1); - private SpelCompiler(ClassLoader classloader) { this.ccl = new ChildClassLoader(classloader); } - /** * Attempt compilation of the supplied expression. A check is * made to see if it is compilable before compilation proceeds. The @@ -131,7 +130,6 @@ public class SpelCompiler implements Opcodes { * @return the expression call, or {@code null} if the decision was to opt out of * compilation during code generation */ - @SuppressWarnings("unchecked") private Class createExpressionClass(SpelNodeImpl expressionToCompile) { // Create class outline 'spel/ExNNN extends org.springframework.expression.spel.CompiledExpression' String clazzName = "spel/Ex" + getNextSuffix(); @@ -183,9 +181,26 @@ public class SpelCompiler implements Opcodes { byte[] data = cw.toByteArray(); // TODO need to make this conditionally occur based on a debug flag // dump(expressionToCompile.toStringAST(), clazzName, data); - return (Class) this.ccl.defineClass(clazzName.replaceAll("/", "."), data); + return loadClass(clazzName.replaceAll("/", "."), data); } + /** + * Load a compiled expression class. Makes sure the classloaders aren't used too much + * because they anchor compiled classes in memory and prevent GC. If you have expressions + * continually recompiling over time then by replacing the classloader periodically + * at least some of the older variants can be garbage collected. + * + * @param name name of the class + * @param bytes bytecode for the class + * @return the Class object for the compiled expression + */ + @SuppressWarnings("unchecked") + private Class loadClass(String name, byte[] bytes) { + if (this.ccl.getClassesDefinedCount() > CLASSES_DEFINED_LIMIT) { + this.ccl = new ChildClassLoader(this.ccl.getParent()); + } + return (Class) this.ccl.defineClass(name, bytes); + } /** * Factory method for compiler instances. The returned SpelCompiler will @@ -269,12 +284,20 @@ public class SpelCompiler implements Opcodes { private static final URL[] NO_URLS = new URL[0]; + private int classesDefinedCount = 0; + public ChildClassLoader(ClassLoader classLoader) { super(NO_URLS, classLoader); } + int getClassesDefinedCount() { + return classesDefinedCount; + } + public Class defineClass(String name, byte[] bytes) { - return super.defineClass(name, bytes, 0, bytes.length); + Class clazz = super.defineClass(name, bytes, 0, bytes.length); + classesDefinedCount++; + return clazz; } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 4d7efd22ef..78edcafdf8 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -22,8 +22,10 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.StringTokenizer; import org.junit.Test; @@ -325,6 +327,25 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals(3.4d, expression.getValue()); } + + @Test + public void repeatedCompilation() throws Exception { + // Verifying that after a number of compilations, the classloaders + // used to load the compiled expressions are discarded/replaced. + // See SpelCompiler.loadClass() + Field f = SpelExpression.class.getDeclaredField("compiledAst"); + Set classloadersUsed = new HashSet<>(); + for (int i=0;i<1500;i++) { // 1500 is greater than SpelCompiler.CLASSES_DEFINED_LIMIT + expression = parser.parseExpression("4 + 5"); + assertEquals(9, (int)expression.getValue(Integer.class)); + assertCanCompile(expression); + f.setAccessible(true); + CompiledExpression cEx = (CompiledExpression)f.get(expression); + classloadersUsed.add(cEx.getClass().getClassLoader()); + assertEquals(9, (int)expression.getValue(Integer.class)); + } + assertTrue(classloadersUsed.size() > 1); + } @SuppressWarnings("rawtypes") @Test