From 702704c3d5ad074271f7decf535205a360722ddf Mon Sep 17 00:00:00 2001 From: adinn Date: Tue, 9 Mar 2010 16:56:02 +0000 Subject: [PATCH] fixed potential strong cyclic reference from proxy cache entries back to their class loader -- fixes JASSIST-104 git-svn-id: http://anonsvn.jboss.org/repos/javassist/trunk@517 30ef5769-5b8d-40dd-aea6-55b5d6557bb3 --- .../javassist/util/proxy/ProxyFactory.java | 133 ++++++++++-------- .../javassist/proxy/ProxyCacheGCTest.java | 116 +++++++++++++++ 2 files changed, 192 insertions(+), 57 deletions(-) create mode 100644 src/test/test/javassist/proxy/ProxyCacheGCTest.java diff --git a/src/main/javassist/util/proxy/ProxyFactory.java b/src/main/javassist/util/proxy/ProxyFactory.java index c6cbb1f5..6f0a1c46 100644 --- a/src/main/javassist/util/proxy/ProxyFactory.java +++ b/src/main/javassist/util/proxy/ProxyFactory.java @@ -22,11 +22,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Member; import java.lang.reflect.Modifier; import java.security.ProtectionDomain; -import java.util.HashMap; -import java.util.WeakHashMap; -import java.util.Iterator; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.lang.ref.WeakReference; import javassist.CannotCompileException; @@ -174,33 +170,70 @@ public class ProxyFactory { private static WeakHashMap proxyCache = new WeakHashMap(); - static class CacheKey { - String classes; + /** + * store details of a specific proxy created using a given method filter and handler + */ + static class ProxyDetails { MethodFilter filter; - private int hash; - WeakReference proxyClass; MethodHandler handler; + Class proxyClass; - public CacheKey(Class superClass, Class[] interfaces, - MethodFilter f, MethodHandler h) + ProxyDetails(MethodFilter filter, MethodHandler handler, Class proxyClass) { - classes = getKey(superClass, interfaces); - hash = classes.hashCode(); - filter = f; - handler = h; - proxyClass = null; + this.filter = filter; + this.handler = handler; + this.proxyClass = proxyClass; } + } - public int hashCode() { return hash; } + /** + * collect together details of all proxies associated with a given classloader which are constructed + * from a given superclass and set of interfaces + */ + static class ProxySet { + String classes; // key constructed from super/interfaces names + List proxyDetails; // hold details of all proxies with given super/interfaces - public boolean equals(Object obj) { - if (obj instanceof CacheKey) { - CacheKey target = (CacheKey)obj; - return target.filter == filter && target.handler == handler - && target.classes.equals(classes); + public ProxySet(String key) + { + classes = key; + proxyDetails = new ArrayList(); + } + + /** + * retrieve an entry from the set with the given filter and handler + * @param filter + * @param handler + * @return the proxy details or null if it is not found + */ + public ProxyDetails lookup(MethodFilter filter, MethodHandler handler) + { + Iterator iterator = proxyDetails.iterator(); + while (iterator.hasNext()) { + ProxyDetails details = (ProxyDetails)iterator.next(); + if (details.filter == filter && details.handler == handler) { + return details; + } } - else - return false; + return null; + } + + /** + * add details of a new proxy to the set + * @param details must not contain the same filter and handler as any existing entry in the set + */ + public void add(ProxyDetails details) + { + proxyDetails.add(details); + } + + /** + * remove details of an existing proxy from the set + * @param details must be in the set + */ + public void remove(ProxyDetails details) + { + proxyDetails.remove(details); } static String getKey(Class superClass, Class[] interfaces) { @@ -283,7 +316,9 @@ public class ProxyFactory { } private void createClass2(ClassLoader cl) { - CacheKey key = new CacheKey(superClass, interfaces, methodFilter, handler); + String key = ProxySet.getKey(superClass, interfaces); + WeakReference reference; + ProxySet set; /* * Excessive concurrency causes a large memory footprint and slows the * execution speed down (with JDK 1.5). Thus, we use a jumbo lock for @@ -294,45 +329,29 @@ public class ProxyFactory { if (cacheForTheLoader == null) { cacheForTheLoader = new HashMap(); proxyCache.put(cl, cacheForTheLoader); - cacheForTheLoader.put(key, key); } - else { - CacheKey found = (CacheKey)cacheForTheLoader.get(key); - if (found == null) - cacheForTheLoader.put(key, key); - else { - key = found; - Class c = isValidEntry(key); // no need to synchronize - if (c != null) { - thisClass = c; - return; - } - } + reference = (WeakReference)cacheForTheLoader.get(key); + if (reference != null) { + set = (ProxySet)reference.get(); + } else { + set = null; } - // } - - // synchronized (key) { - Class c = isValidEntry(key); - if (c == null) { + if (set == null) { + set = new ProxySet(key); + reference = new WeakReference(set); + cacheForTheLoader.put(key, reference); + } + ProxyDetails details = set.lookup(methodFilter, handler); + if (details == null) { createClass3(cl); - key.proxyClass = new WeakReference(thisClass); + details = new ProxyDetails(methodFilter, handler, thisClass); + set.add(details); + } else { + thisClass = details.proxyClass; } - else - thisClass = c; // } } - private Class isValidEntry(CacheKey key) { - WeakReference ref = key.proxyClass; - if (ref != null) { - Class c = (Class)ref.get(); - if(c != null) - return c; - } - - return null; - } - private void createClass3(ClassLoader cl) { try { ClassFile cf = make(); diff --git a/src/test/test/javassist/proxy/ProxyCacheGCTest.java b/src/test/test/javassist/proxy/ProxyCacheGCTest.java new file mode 100644 index 00000000..8f583a37 --- /dev/null +++ b/src/test/test/javassist/proxy/ProxyCacheGCTest.java @@ -0,0 +1,116 @@ +package test.javassist.proxy; + +import javassist.*; +import javassist.util.proxy.MethodFilter; +import javassist.util.proxy.MethodHandler; +import javassist.util.proxy.ProxyFactory; +import junit.framework.TestCase; + +/** + * test which checks that proxy classes are not retained after their classloader is released. + * this is a before and after test which validates JASSIST-104 + */ +public class ProxyCacheGCTest extends TestCase +{ + /** + * creates a large number of proxies in separate classloaders then lets go of the classloaders and + * forces a GC. If we run out of heap then we know there is a problem. + */ + + public final static int REPETITION_COUNT = 10000; + private ClassPool basePool; + private CtClass baseHandler; + private CtClass baseFilter; + + protected void setUp() + { + basePool = ClassPool.getDefault(); + try { + baseHandler = basePool.get("javassist.util.proxy.MethodHandler"); + baseFilter = basePool.get("javassist.util.proxy.MethodFilter"); + } catch (NotFoundException e) { + e.printStackTrace(); + fail("could not find class " + e); + } + } + + public void testCacheGC() + { + ClassLoader oldCL = Thread.currentThread().getContextClassLoader(); + ProxyFactory.useCache = false; + for (int i = 0; i < REPETITION_COUNT; i++) { + ClassLoader newCL = new TestLoader(); + try { + Thread.currentThread().setContextClassLoader(newCL); + createProxy(i); + } finally { + Thread.currentThread().setContextClassLoader(oldCL); + } + } + } + + /** + * called when a specific classloader is in place on the thread to create a method handler, method filter + * and proxy in the current loader and then + */ + public void createProxy(int counter) + { + try { + ClassPool classPool = new ClassPool(basePool); + + // create a target class in the current class loader + String targetName = "test.javassist.MyTarget_" + counter; + String targetConstructorName = "MyTarget_" + counter; + CtClass ctTargetClass = classPool.makeClass(targetName); + CtMethod targetMethod = CtNewMethod.make("public Object test() { return this; }", ctTargetClass); + ctTargetClass.addMethod(targetMethod); + CtConstructor targetConstructor = CtNewConstructor.make("public " + targetConstructorName + "() { }", ctTargetClass); + ctTargetClass.addConstructor(targetConstructor); + + // create a handler in the current classloader + String handlerName = "test.javassist.MyHandler_" + counter; + CtClass ctHandlerClass = classPool.makeClass(handlerName); + ctHandlerClass.addInterface(baseHandler); + CtMethod handlerInvoke = CtNewMethod.make("public Object invoke(Object self, java.lang.reflect.Method thisMethod, java.lang.reflect.Method proceed, Object[] args) throws Throwable { return proceed.invoke(self, args); }", ctHandlerClass); + ctHandlerClass.addMethod(handlerInvoke); + + // create a filter in the current classloader + String filterName = "test.javassist.MyFilter" + counter; + CtClass ctFilterClass = classPool.makeClass(filterName); + ctFilterClass.addInterface(baseFilter); + CtMethod filterIsHandled = CtNewMethod.make("public boolean isHandled(java.lang.reflect.Method m) { return true; }", ctFilterClass); + ctFilterClass.addMethod(filterIsHandled); + + // now create a proxyfactory and use it to create a proxy + + ProxyFactory factory = new ProxyFactory(); + Class javaTargetClass = classPool.toClass(ctTargetClass); + Class javaHandlerClass = classPool.toClass(ctHandlerClass); + Class javaFilterClass = classPool.toClass(ctFilterClass); + + MethodHandler handler= (MethodHandler)javaHandlerClass.newInstance(); + MethodFilter filter = (MethodFilter)javaFilterClass.newInstance(); + + // ok, now create a factory and a proxy class and proxy from that factory + factory.setHandler(handler); + factory.setFilter(filter); + factory.setSuperclass(javaTargetClass); + // factory.setSuperclass(Object.class); + + Class proxyClass = factory.createClass(); + Object target = proxyClass.newInstance(); + } catch (Exception e) { + e.printStackTrace(); + fail("cannot create proxy " + e); + } + + } + + /** + * a classloader which inherits from the system class loader and within which a proxy handler, + * filter and proxy will be located. + */ + public static class TestLoader extends ClassLoader + { + } +} -- 2.39.5