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

Memory leak when using annotation based auto-wiring in child context [SPR-11520] #16145

Closed
spring-projects-issues opened this issue Mar 5, 2014 · 10 comments
Assignees
Labels
in: core status: backported type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Mar 5, 2014

Robert Cohen opened SPR-11520 and commented

I have a Spring Context hierarchy that is two levels where each node has it's own classloader. I load the child context using an XML bean definition and as an option, there may be a context:component-scan defined within the child context's XML to enable annotation style auto-wiring. When we have this component-scan, the child classloader is not garbage collected if the child context is closed. If we use XML for all of our bean definitions, then the classloader is collected properly.

The attached test project reproduces the leak consistently after creating/closing a child context about 318 times.

Using a profiler, I can see that the child classloader is being held due to child context Classes being left in the Parent's DefaultListableBeanFactory "allBeanNamesByType" variable:

/** Map of singleton and non-singleton bean names keyed by dependency type */
private final Map<Class<?>, String[]> allBeanNamesByType = new ConcurrentHashMap<Class<?>, String[]>(64);

They are added to the Parent DefaultListableBeanFactory allBeanNamesByType map by method DefaultListableBeanFactory. getBeanNamesForType(), however they are not removed when I close the context.

As the server is used an child contexts get redeployed, we eventually encounter OOM errors.


Affects: 3.2.8

Attachments:

Issue Links:

  • #16229 AbstractApplicationEventMulticaster can leak classes
  • #16494 Performance regression for custom autowireBean calls with many properties
  • #21255 Revise cache safety check to avoid performance regression in EAR packaged applications on WildFly

Backported to: 3.2.9

0 votes, 6 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2014

Juergen Hoeller commented

What kind of Class references can you see in that parent's allBeanNamesByType Map? Why are they different between the component-scan vs XML case?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2014

Robert Cohen commented

The Class references in the parent's allBeanNamesByType Map are any child classes (annotated with @Component) that have been auto-wired. In the example code the Child context contains @Component's Crib, Child and Toy. Toy gets wired to Child and Child gets wired to Crib. In the parent's allBeanNamesByType map, i see references to Child.class and Toy.class, but not Crib.class.

The following stack trace shows when one of the child classes gets added to the parent's allBeanNamesByType map:

Thread [main] (Suspended (breakpoint at line 335 in DefaultListableBeanFactory))	
	owns: ConcurrentHashMap<K,V>  (id=25)	
	owns: Object  (id=26)	
	DefaultListableBeanFactory.getBeanNamesForType(Class<?>, boolean, boolean) line: 335	
	BeanFactoryUtils.beanNamesForTypeIncludingAncestors(ListableBeanFactory, Class<?>, boolean, boolean) line: 187	
	BeanFactoryUtils.beanNamesForTypeIncludingAncestors(ListableBeanFactory, Class<?>, boolean, boolean) line: 191	
	DefaultListableBeanFactory.findAutowireCandidates(String, Class<?>, DependencyDescriptor) line: 897	
	DefaultListableBeanFactory.doResolveDependency(DependencyDescriptor, Class<?>, String, Set<String>, TypeConverter) line: 855	
	DefaultListableBeanFactory.resolveDependency(DependencyDescriptor, String, Set<String>, TypeConverter) line: 770	
	AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(Object, String, PropertyValues) line: 489	
	InjectionMetadata.inject(Object, String, PropertyValues) line: 87	
	AutowiredAnnotationBeanPostProcessor.postProcessPropertyValues(PropertyValues, PropertyDescriptor[], Object, String) line: 286	
	DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).populateBean(String, RootBeanDefinition, BeanWrapper) line: 1146	
	DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).doCreateBean(String, RootBeanDefinition, Object[]) line: 519	
	DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).createBean(String, RootBeanDefinition, Object[]) line: 458	
	AbstractBeanFactory$1.getObject() line: 296	
	DefaultListableBeanFactory(DefaultSingletonBeanRegistry).getSingleton(String, ObjectFactory<?>) line: 223	
	DefaultListableBeanFactory(AbstractBeanFactory).doGetBean(String, Class<T>, Object[], boolean) line: 293	
	DefaultListableBeanFactory(AbstractBeanFactory).getBean(String) line: 194	
	DefaultListableBeanFactory.preInstantiateSingletons() line: 628	
	GenericApplicationContext(AbstractApplicationContext).finishBeanFactoryInitialization(ConfigurableListableBeanFactory) line: 932	
	GenericApplicationContext(AbstractApplicationContext).refresh() line: 479	
	Main.main(String[]) line: 39	

Specifically, here in DefaultListableBeanFactory:

public String[] getBeanNamesForType(Class<?> type, boolean includeNonSingletons, boolean allowEagerInit) {
     if (!isConfigurationFrozen()  || type == null || !allowEagerInit) {
          return doGetBeanNamesForType(type, includeNonSingletons, allowEagerInit);
     }
     Map<Class<?>, String[]> cache =
               (includeNonSingletons ? this.allBeanNamesByType : this.singletonBeanNamesByType);
     String[] resolvedBeanNames = cache.get(type);
     if (resolvedBeanNames != null) {
          return resolvedBeanNames;
     }
     resolvedBeanNames = doGetBeanNamesForType(type, includeNonSingletons, allowEagerInit);
     cache.put(type, resolvedBeanNames);
     return resolvedBeanNames;
}

It's interesting because first this method is called on the child context and the bean names being looked up are found and cached in the child. But then this logic is also called on the parent context which creates the issue.

If i remove the component scan and define the beans in XML, then BeanFactoryUtils.beanNamesForTypeIncludingAncestors() does not get called and the child class references are never added to the parent allBeanNamesByType map.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2014

Juergen Hoeller commented

Thanks for sharing those details...

So it looks like the actual cause is the use of autowiring (@Autowired and co), not the use of component scanning. You'd probably see the same effect if you took your annotated components as-is but defined them in <bean class="..."/> one-liners instead of using context:component-scan.

We're indeed caching lookup results at all factory levels, independent from where those classes originate. I suppose we need to either use weak references there or clear the parent's cache if a child factory is being closed... covering scenarios like yours where child factories are being closed and reopened while the parent stays alive all the time.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2014

Robert Cohen commented

I tried removing the component scan and adding the bean one liners and confirmed that I still see the leak. So I have changed the title of the issue to reflect this is and issue with annotation based auto-wiring.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2014

Robert Cohen commented

Additionally, I can confirm that using a soft reference map for allBeanNamesByType does fix the leak. However, I'm am not the best to determine the consequences of this change.

private final Map<Class<?>, String[]> allBeanNamesByType = new ConcurrentReferenceHashMap<Class<?>, String[]>(64, ConcurrentReferenceHashMap.ReferenceType.SOFT);

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 6, 2014

Juergen Hoeller commented

I eventually opted for a different approach:

DefaultListableBeanFactory only puts 'cache-safe' Class keys into its by-type cache now, as determined by our ClassUtils.isCacheSafe method which is also being used by our JavaBeans CachedIntrospectionResults.

Effectively, DefaultListableBeanFactory will now only cache retrieval results for classes loaded by that factory's bean ClassLoader or a parent thereof, ignoring classes from child ClassLoaders completely.

In a regular scenario, this won't make a difference since the entire application typically lives within the same ClassLoader anyway. However, in a scenario like yours, it will prevent memory leaks through (always empty but nevertheless cached) retrieval results in a parent factory.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 7, 2014

Juergen Hoeller commented

This is available in the latest 4.0.3 snapshot already, and will be available in the immediately upcoming 3.2.9 snapshot as well (see http://projects.spring.io/spring-framework/ for Maven coordinates). Please give it a try and let us know whether it works for you...

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 7, 2014

Robert Cohen commented

My project uses 3.2.6 so I patched that version with your fix and tested it. It works perfectly to solve the problem. Thanks a ton for the quick turnaround!
-Rob

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 21, 2014

Matthias Müller commented

Wow, this was a hard nut to crack, I had the exact problem in my application.
I was looking for a selfmade memory leak in the first place before I found this ticket.
I had a DefaultListableBeanFactory consuming ~700MB of Memory....but now everthing works fine!
Tnx for finding and fixing this!

@tsinicki
Copy link

@tsinicki tsinicki commented Dec 18, 2019

After browsing through the issues related to the bean name caching I though this one here might be a good one to leave a comment.

As far as we understood the isCacheSafe method of the ClassUtils was modified because of this issue to check whether a particular class can be loaded through its beanclassloader (or one of its parents) or not. If the class can be loaded through the classloader it is going to be cached and in case it cannot be loaded there is no caching.

Related to this we’ve noticed some performance problems linked to request scoped beans. Because the information that the bean type cannot be resolved by one of the parents it is not cached, spring is doing the lookup for each request again which slows down the performance measurably. To solve the issue we’re currently caching an empty string array if the parent classloader is not able to load a particular type of class.

Please let us know what you think regarding this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: backported type: bug
Projects
None yet
Development

No branches or pull requests

3 participants