You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by jw...@apache.org on 2016/01/24 15:52:28 UTC
groovy git commit: GROOVY-4698 - Possible memory leak in Tomcat
(GroovyScriptEngine ThreadLocal) (closes #245)
Repository: groovy
Updated Branches:
refs/heads/master db29c7d9a -> 428dad537
GROOVY-4698 - Possible memory leak in Tomcat (GroovyScriptEngine ThreadLocal) (closes #245)
GSE uses a ThreadLocal to temporarily cache data so it can be used in the compilation phase. Once the script is compiled the cache data is no longer necessary. ThreadLocal.set(null) was used which leaves the ThreadLocal as a key in the Thread's ThreadLocalMap entries. In addition, if any exceptions were thrown before that call it would leave a ThreadLocal entry with a LocalData value in the thread's ThreadLocalMap tables. This change tries to ensure that the ThreadLocal is removed when no longer needed.
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/428dad53
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/428dad53
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/428dad53
Branch: refs/heads/master
Commit: 428dad537d5ce63a5af52cd2bca77e328bf8a2df
Parents: db29c7d
Author: John Wagenleitner <jw...@apache.org>
Authored: Sun Jan 24 06:39:51 2016 -0800
Committer: John Wagenleitner <jw...@apache.org>
Committed: Sun Jan 24 06:39:51 2016 -0800
----------------------------------------------------------------------
src/main/groovy/util/GroovyScriptEngine.java | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/428dad53/src/main/groovy/util/GroovyScriptEngine.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/util/GroovyScriptEngine.java b/src/main/groovy/util/GroovyScriptEngine.java
index a9ccb72..cbb3159 100644
--- a/src/main/groovy/util/GroovyScriptEngine.java
+++ b/src/main/groovy/util/GroovyScriptEngine.java
@@ -244,13 +244,25 @@ public class GroovyScriptEngine implements ResourceConnector {
}
}
- private Class doParseClass(GroovyCodeSource codeSource) {
+ private Class<?> doParseClass(GroovyCodeSource codeSource) {
// local is kept as hard reference to avoid garbage collection
ThreadLocal<LocalData> localTh = getLocalData();
LocalData localData = new LocalData();
localTh.set(localData);
StringSetMap cache = localData.dependencyCache;
+ Class<?> answer = null;
+ try {
+ updateLocalDependencyCache(codeSource, localData);
+ answer = super.parseClass(codeSource, false);
+ updateScriptCache(localData);
+ } finally {
+ cache.clear();
+ localTh.remove();
+ }
+ return answer;
+ }
+ private void updateLocalDependencyCache(GroovyCodeSource codeSource, LocalData localData) {
// we put the old dependencies into local cache so createCompilationUnit
// can pick it up. We put that entry under the name "."
ScriptCacheEntry origEntry = scriptCache.get(codeSource.getName());
@@ -268,11 +280,13 @@ public class GroovyScriptEngine implements ResourceConnector {
}
}
+ StringSetMap cache = localData.dependencyCache;
cache.put(".", newDep);
}
+ }
- Class answer = super.parseClass(codeSource, false);
-
+ private void updateScriptCache(LocalData localData) {
+ StringSetMap cache = localData.dependencyCache;
cache.makeTransitiveHull();
long time = getCurrentTime();
Set<String> entryNames = new HashSet<String>();
@@ -294,9 +308,6 @@ public class GroovyScriptEngine implements ResourceConnector {
ScriptCacheEntry cacheEntry = new ScriptCacheEntry(clazz, lastModified, time, value, false);
scriptCache.put(entryName, cacheEntry);
}
- cache.clear();
- localTh.set(null);
- return answer;
}
private String getPath(Class clazz, Map<String, String> precompiledEntries) {