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

MemoryLeak in Cglib2AopProxy.ProxyCallbackFilter [SPR-8008] #12663

Closed
spring-projects-issues opened this issue Mar 1, 2011 · 10 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 1, 2011

Tommy Becker opened SPR-8008 and commented

We are having issues with OOM errors during unit test runs. The runs setup and subsequently tear down a lot of Spring contexts, and we're noticing the BeanFactory instances are not getting cleaned up. Analysis of heap dumps shows that the Cglib2AopProxy.ProxyCallbackFilter, which is cached by cglib, maintains a chain of strong references back to the BeanFactory. Did some searching and it looks like the same type of problem as described in this older issue:

https://jira.springsource.org/browse/SPR-3620

Attached is a graphic showing the reference chain.


Affects: 3.0.3

Reference URL: http://forum.springsource.org/showthread.php?t=104193

Attachments:

Issue Links:

Backported to: 3.1.4

8 votes, 6 watchers

@spring-projects-issues
Copy link
Collaborator Author

Costin Leau commented

What version of cglib are you using?

@spring-projects-issues
Copy link
Collaborator Author

Tommy Becker commented

cglib 2.2. That's the latest, correct?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 14, 2011

Chris Beams commented

This is a familiar-looking issue, indeed. Relates to #12556, #8302, #8684.

@spring-projects-issues
Copy link
Collaborator Author

Scott Hamilton commented

FWIW, I resolved this by injecting my own version of the CglibSubclassingInstantiationStrategy into the application context. This one does not maintain references to objects that directly link to the application context, but rather does an on-demand lookup of the bean factory from a global/static utility class we have in our application. Instead of passing the RootBeanDefinition through to the CglibSubclassCreator, we pass in the beanName and beanClass, which is really all that needs regarding the bean itself. Using that info plus our lookup util to get the BeanFactory, I rewrote the remaining inner classes (the MethodInterceptors) to do the on-demand lookup of the AbstractBeanDefinition from the factory using the beanName.

While our code is probably not directly applicable to a long-term fix, the concept might help. It certainly resolved the issue for us (as it involved this particular cglib leak, anyway).

@spring-projects-issues
Copy link
Collaborator Author

Peter Ertl commented

this issue is pretty old now and we run into the same problem. our tests are producing OOME like crazy. any plans on fixing this soon?

@spring-projects-issues
Copy link
Collaborator Author

Roman Kalaputs commented

Still reproducible on Spring 3.1.0 + CGLIB 2.2.2. Steps to reproduce are the same as mentioned in desciption - refresh and close AppContext several times.

P.S. Make sure that you are using Spring AOP :)

@spring-projects-issues
Copy link
Collaborator Author

Roman Kalaputs commented

Let me add more information about this problem.

The main reason:
CallbackFilter is a part of KEY which is used by CGLIB Enhancer for storing generated proxies in the cache for future usage within same ClassLoader. The cache itself is a HahMap. The hashcode of the KEY is calculated based on hashcode of its fields, so as result instance of CallbackFilter (ProxyCallbackFilter) affects it.

The current implementation of ProxyCallbackFilter.hashCode method:

public int hashCode() {
	int hashCode = 0;
	Advisor[] advisors = this.advised.getAdvisors();
	for (Advisor advisor : advisors) {
		Advice advice = advisor.getAdvice();
		if (advice != null) {
			hashCode = 13 * hashCode + advice.getClass().hashCode();
		}
	}
	hashCode = 13 * hashCode + (this.advised.isFrozen() ? 1 : 0);
	hashCode = 13 * hashCode + (this.advised.isExposeProxy() ? 1 : 0);
	hashCode = 13 * hashCode + (this.advised.isOptimize() ? 1 : 0);
	hashCode = 13 * hashCode + (this.advised.isOpaque() ? 1 : 0);
	return hashCode;
}

depends on list of Advices for object which is being proxied.

For some standard advices e.g. TransactionInterceptor method "hashCode" is not overrided (I guess it's correct) as result it depends on Advice instance which is recreated each time context loaded.

So such proxies always have different hash code each time context is re-created. As result previous instances cannot be reused and CGLIB cache growing until OOM.

Now I am investigating how to fix it properly (except disabling CGLIB cache)

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Roman, we're not actually using the Advice instance hashCode there... The calculation in ProxyCallbackFilter is rather based on the Advice class hashCode, unless I'm missing something?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Tommy Becker commented

I think the implementation of https://jira.springsource.org/browse/SPR-6184 increases the impact of this issue. The reason being that the new behavior of AnnotationConfigContextLoader encourages tests with inner @Configuration classes. This effectively defeats the TestContext framework's context caching mechanism (since you're creating a context exclusively for that test), which in turn means a lot more contexts are being setup and then torn down. At this point we are having to run a JVM per test in some cases to avoid the OOM errors.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I've fixed several equals/hashCode in pointcut implementations and some delegate classes thereof now. In my tests, all regular scenarios - including usage of the AOP namespace and transaction annotations - work fine now.

This is basically improving the reuse of CGLIB proxy classes across context refreshes. There might still be some callback filters with external references cached but CGLIB's proxy class cache is not filling up anymore.

This will be released in 3.2 GA (due tomorrow) and 3.1.4 (following soon).

Juergen

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: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants