From dd8d42afa0f129e439ea5026b051a496794ab1f5 Mon Sep 17 00:00:00 2001 From: Jean-Louis Monteiro Date: Fri, 9 May 2025 13:46:11 +0200 Subject: [PATCH 1/2] fix(#OWB-1450): Interceptor proxy memory leak --- .../InterceptorDecoratorProxyFactory.java | 59 +++++++++++++++++-- .../InterceptorDecoratorProxyFactoryTest.java | 41 +++++++++++++ 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/webbeans-impl/src/main/java/org/apache/webbeans/proxy/InterceptorDecoratorProxyFactory.java b/webbeans-impl/src/main/java/org/apache/webbeans/proxy/InterceptorDecoratorProxyFactory.java index 8dfedfaea..b11e43aaa 100644 --- a/webbeans-impl/src/main/java/org/apache/webbeans/proxy/InterceptorDecoratorProxyFactory.java +++ b/webbeans-impl/src/main/java/org/apache/webbeans/proxy/InterceptorDecoratorProxyFactory.java @@ -23,6 +23,7 @@ import org.apache.webbeans.exception.ProxyGenerationException; import org.apache.webbeans.exception.WebBeansConfigurationException; import org.apache.webbeans.intercept.InterceptorResolutionService; +import org.apache.webbeans.portable.AnnotatedTypeImpl; import org.apache.webbeans.util.Asserts; import org.apache.webbeans.util.ExceptionUtil; import org.apache.xbean.asm9.ClassWriter; @@ -39,12 +40,13 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.Collection; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; /** * Generate a dynamic subclass which has exactly 1 delegation point instance - * which get's set in the Constructor of the proxy. + * which gets set in the Constructor of the proxy. * Any non-intercepted or decorated method will get delegated natively, * All intercepted and decorated methods will get invoked via an InvocationHandler chain. * @@ -67,7 +69,7 @@ public class InterceptorDecoratorProxyFactory extends AbstractProxyFactory * We need this to prevent filling up the ClassLoaders by */ private ConcurrentMap, Class> cachedProxyClasses = new ConcurrentHashMap<>(); - private ConcurrentMap, Class> cachedProxyClassesByAt = new ConcurrentHashMap<>(); + private ConcurrentMap> cachedProxyClassesByAt = new ConcurrentHashMap<>(); public InterceptorDecoratorProxyFactory(WebBeansContext webBeansContext) @@ -191,7 +193,7 @@ public synchronized Class createProxyClass(InterceptorResolutionService.B Class proxyClass = createProxyClass( classLoader, at.getJavaClass(), intercepted.toArray(new Method[intercepted.size()]), others.toArray(new Method[others.size()])); - cachedProxyClassesByAt.put(at, proxyClass); + cachedProxyClassesByAt.put(new AnnotationTypeKey(at), proxyClass); return proxyClass; } @@ -224,7 +226,7 @@ private Class createProxyClass(ClassLoader classLoader, Class classToP public Class getCachedProxyClass(InterceptorResolutionService.BeanInterceptorInfo interceptorInfo, AnnotatedType at, ClassLoader classLoader) { - Class value = (Class) cachedProxyClassesByAt.get(at); + Class value = (Class) cachedProxyClassesByAt.get(new AnnotationTypeKey(at)); if (value == null) { value = createProxyClass(interceptorInfo, at, classLoader); @@ -613,5 +615,54 @@ else if (methodIndex < 32267) mv.visitEnd(); } + public static class AnnotationTypeKey { + private final AnnotatedType annotatedType; + + + public AnnotationTypeKey(final AnnotatedType annotatedType) + { + Asserts.assertNotNull(annotatedType, "annotatedType"); + this.annotatedType = annotatedType; + } + + @Override + public final boolean equals(final Object o) + { + if (!(o instanceof AnnotationTypeKey)) + { + return false; + } + + final AnnotatedType at = ((AnnotationTypeKey) o).annotatedType; + if (annotatedType == at) + { + return true; + } + + if (annotatedType.equals(at)) + { + return true; + } + + if (annotatedType.getJavaClass() == at.getJavaClass()) + { + return true; + } + + return false; + } + + @Override + public int hashCode() + { + if (annotatedType instanceof AnnotatedTypeImpl) + { + final var at = (AnnotatedTypeImpl) annotatedType; + return at.getJavaClass().hashCode(); + + } + return Objects.hashCode(annotatedType); + } + } } diff --git a/webbeans-impl/src/test/java/org/apache/webbeans/test/interceptors/factory/InterceptorDecoratorProxyFactoryTest.java b/webbeans-impl/src/test/java/org/apache/webbeans/test/interceptors/factory/InterceptorDecoratorProxyFactoryTest.java index dd6f8d6ff..9dfcc53ac 100644 --- a/webbeans-impl/src/test/java/org/apache/webbeans/test/interceptors/factory/InterceptorDecoratorProxyFactoryTest.java +++ b/webbeans-impl/src/test/java/org/apache/webbeans/test/interceptors/factory/InterceptorDecoratorProxyFactoryTest.java @@ -28,11 +28,15 @@ import java.net.URL; import java.net.URLClassLoader; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Set; import org.apache.webbeans.config.WebBeansContext; +import org.apache.webbeans.configurator.AnnotatedTypeConfiguratorImpl; import org.apache.webbeans.exception.WebBeansException; +import org.apache.webbeans.intercept.InterceptorResolutionService; +import org.apache.webbeans.portable.AnnotatedTypeImpl; import org.apache.webbeans.test.AbstractUnitTest; import org.apache.webbeans.test.component.intercept.webbeans.TransactionalInterceptor; import org.apache.webbeans.test.interceptors.factory.beans.ClassInterceptedClass; @@ -41,15 +45,19 @@ import org.apache.webbeans.proxy.InterceptorHandler; import org.apache.webbeans.proxy.OwbInterceptorProxy; import org.apache.webbeans.test.interceptors.factory.beans.TonsOfMethodsInterceptedClass; +import org.apache.webbeans.util.Asserts; import org.apache.webbeans.util.ClassUtil; import org.apache.webbeans.test.util.CustomBaseType; import org.apache.webbeans.test.util.CustomType; import org.apache.webbeans.test.util.ExtendedSpecificClass; import org.apache.webbeans.test.util.GenericInterface; import org.apache.webbeans.test.util.SpecificClass; +import org.apache.webbeans.util.WebBeansUtil; import org.junit.Assert; import org.junit.Test; +import static org.junit.Assert.assertSame; + /** * Test the {@link org.apache.webbeans.proxy.InterceptorDecoratorProxyFactory} @@ -57,6 +65,39 @@ public class InterceptorDecoratorProxyFactoryTest extends AbstractUnitTest { + @Test + public void testEnsureOneProxyPerAT() + { + // we take a fresh URLClassLoader to not blur the test classpath with synthetic classes. + ClassLoader classLoader = new URLClassLoader(new URL[0]); + + final WebBeansContext context = WebBeansContext.currentInstance(); + final InterceptorDecoratorProxyFactory factory = context.getInterceptorDecoratorProxyFactory(); + + // create first annotated type and it's associated proxy + final var at = new AnnotatedTypeImpl(context, context.getBeanManagerImpl().createAnnotatedType(DummyBean.class)); + final var configurator = new AnnotatedTypeConfiguratorImpl(context, at); + final AnnotatedTypeImpl newAnnotatedType = configurator.getNewAnnotatedType(); + final InterceptorResolutionService.BeanInterceptorInfo interceptorInfo = context.getInterceptorResolutionService() + .calculateInterceptorInfo(newAnnotatedType.getTypeClosure(), Collections.emptySet(), newAnnotatedType, true); + final Class subClass = factory.getCachedProxyClass(interceptorInfo, newAnnotatedType, classLoader); + + Asserts.assertNotNull(subClass); + + // create second annotated type and it's associated proxy + final var at2 = new AnnotatedTypeImpl(context, context.getBeanManagerImpl().createAnnotatedType(DummyBean.class)); + final var configurator2 = new AnnotatedTypeConfiguratorImpl(context, at2); + final AnnotatedTypeImpl newAnnotatedType2 = configurator2.getNewAnnotatedType(); + final InterceptorResolutionService.BeanInterceptorInfo interceptorInfo2 = context.getInterceptorResolutionService() + .calculateInterceptorInfo(newAnnotatedType2.getTypeClosure(), Collections.emptySet(), newAnnotatedType2, true); + final Class subClass2 = factory.getCachedProxyClass(interceptorInfo2, newAnnotatedType2, classLoader); + + Asserts.assertNotNull(subClass2); + + // the 2 proxy instances should be the same and the cache in the factory should be filed in with one proxy only + assertSame(subClass, subClass2); + } + @Test public void testSimpleProxyCreation() throws Exception { From f81d0540aac7d9b5497cd776be025f895dd17ce7 Mon Sep 17 00:00:00 2001 From: Jean-Louis Monteiro Date: Fri, 9 May 2025 13:55:58 +0200 Subject: [PATCH 2/2] style: checkstyle --- .../webbeans/proxy/InterceptorDecoratorProxyFactory.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webbeans-impl/src/main/java/org/apache/webbeans/proxy/InterceptorDecoratorProxyFactory.java b/webbeans-impl/src/main/java/org/apache/webbeans/proxy/InterceptorDecoratorProxyFactory.java index b11e43aaa..89862fbd9 100644 --- a/webbeans-impl/src/main/java/org/apache/webbeans/proxy/InterceptorDecoratorProxyFactory.java +++ b/webbeans-impl/src/main/java/org/apache/webbeans/proxy/InterceptorDecoratorProxyFactory.java @@ -615,7 +615,8 @@ else if (methodIndex < 32267) mv.visitEnd(); } - public static class AnnotationTypeKey { + public static class AnnotationTypeKey + { private final AnnotatedType annotatedType;