Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1st instance of AOP proxy leaks its advice #25505

Closed
basi01 opened this issue Aug 1, 2020 · 1 comment
Closed

1st instance of AOP proxy leaks its advice #25505

basi01 opened this issue Aug 1, 2020 · 1 comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another

Comments

@basi01
Copy link

basi01 commented Aug 1, 2020

Affects: 5.2.8.RELEASE

A programmer supplies an implementation of org.aopalliance.aop.Advice to ProxyFactory.addAdvice(). A custom advice may occupy a lot of memory, nobody forbids that. Then ProxyFactory.getProxy() will generate a class for the current factory config and put it in a cache:

	at org.springframework.cglib.core.internal.LoadingCache.get(LoadingCache.java:28)
	at org.springframework.cglib.core.AbstractClassGenerator$ClassLoaderData.get(AbstractClassGenerator.java:134)
	at org.springframework.cglib.core.AbstractClassGenerator.create(AbstractClassGenerator.java:319)
	at org.springframework.cglib.proxy.Enhancer.createHelper(Enhancer.java:572)
	at org.springframework.cglib.proxy.Enhancer.createClass(Enhancer.java:419)
	at org.springframework.aop.framework.ObjenesisCglibAopProxy.createProxyClassAndInstance(ObjenesisCglibAopProxy.java:57)
	at org.springframework.aop.framework.CglibAopProxy.getProxy(CglibAopProxy.java:205)
	at org.springframework.aop.framework.CglibAopProxy.getProxy(CglibAopProxy.java:155)
	at org.springframework.aop.framework.ProxyFactory.getProxy(ProxyFactory.java:97)

Both key and value weakly refer to the custom Advice, however, there's a strong reference to the ProxyCallbackFilter from the generated class (I couldn't determine the exact field in the generated class, but Eclipse shows there's a reference)

And this class is loaded in the application classloader and will not be collected. I think this is why the initial Advice instances are not collected.

There's no reason for the generated class to refer the original advices because when a new proxy is created, new advice instances are used even if their classes are the same. Also, I think maybe we shouldn't store the custom advices as part of the key. ProxyCallbackFilter only compares the advice classes and the advisor pointcuts in its equals() method.

Here's the test program:

    package org.foo.testproxy;
    
    import java.lang.ref.WeakReference;
    import java.util.ArrayList;
    import java.util.List;
    import java.util.Objects;
    
    import org.aopalliance.intercept.MethodInterceptor;
    import org.aopalliance.intercept.MethodInvocation;
    import org.springframework.aop.Advisor;
    import org.springframework.aop.framework.ProxyFactory;
    
    public class TestGC {
    
        public static void main(String[] args) throws Exception {
            doStuff();
            doStuff();
            gc();
            if (proxyRef.get() != null) throw new Exception("unexpected");
            if (factoryRef.get() != null) throw new Exception("unexpected");
            Advisor advisor = Objects.requireNonNull(advisorRefs.get(0).get());
            System.out.println("advisor leaked");
            advisor.getClass(); // breakpoint here
        }
    
        static List<WeakReference<Advisor>> advisorRefs = new ArrayList<>();
        static WeakReference<List<Advisor>> advisorsRef;
        static WeakReference<ProxyFactory> factoryRef;
        static WeakReference<Object> proxyRef;
    
        static void doStuff() {
            A obj = new A();
            ProxyFactory factory = new ProxyFactory(obj) {
                {
                    advisorsRef = new WeakReference<>( getAdvisorsInternal() );
                }
            };
            factory.addAdvice(new TestInterceptor());
            advisorRefs.add( new WeakReference<>( factory.getAdvisors()[0]) );
            factoryRef = new WeakReference<>( factory );
            proxyRef =  new WeakReference<>( factory.getProxy() );
        }
        
        static class A {
        }
        
        static class TestInterceptor implements MethodInterceptor {
            public Object invoke(MethodInvocation invocation) throws Throwable {
                return invocation.proceed();
            }
        }
        
        private static void gc() {
            for (int i = 0; i < 100; i++) {
                (new byte[1000000]).getClass();
                System.gc();
            }
        }
    }
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 1, 2020
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Aug 10, 2020
@sbrannen sbrannen changed the title 1st instance of aop proxy leaks its advices 1st instance of AOP proxy leaks its advice Aug 10, 2020
@sbrannen
Copy link
Member

This appears to have been superseded by the following.

In light of that, I am closing this issue.

However, if you find that still encounter issues with Spring Framework 6.0.10+, please let us know.

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Aug 27, 2023
@sbrannen sbrannen added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

3 participants