Concurrent creation of the same Configuration class in different contexts is not thread-safe [SPR-10307]

[spring-projects-issues]

Vivien HENRIET opened SPR-10307 and commented

The refresh() method of AnnotationConfigApplicationContext is not thread safe anymore.
It works fine in 3.1.2. The regression has bean introduce with the 3.1.3 release.

To reproduce:
-create 2 application contexts
-register the same configuration class
-call the refresh method on application context simultaneously

Now, there is a good chance that both application context share the same beans.

A small Java program is attached to reproduce the issue.

---

Affects: 3.1.3, 3.1.4, 3.2.1, 3.2.2

Attachments:

• threadsafe.zip (2.46 kB)

Issue Links:

• Use of same @Configuration class across multiple ApplicationContexts is not threadsafe [SPR-10936] #15564 Use of same @Configuration class across multiple ApplicationContexts is not threadsafe ("is duplicated by")
• Singleton-scoped @Bean methods behave like prototypes in a Spring DM environment [SPR-5932] #10601 Singleton-scoped @Bean methods behave like prototypes in a Spring DM environment
• Prevent memory leaks with @Configuration beans [SPR-9851] #14484 Prevent memory leaks with @Configuration beans
• Java config FactoryBean proxy causes class loader leak [SPR-9274] #13912 Java config FactoryBean proxy causes class loader leak
• Creating ApplicationContexts from multiple threads causes ConcurrentModificationException [SPR-10543] #15173 Creating ApplicationContexts from multiple threads causes ConcurrentModificationException

Referenced from: commits 30b21a9, 6405551, fffeaee

0 votes, 6 watchers

[spring-projects-issues]

Juergen Hoeller commented

After some initial research, it seems that this is caused by CGLIB's internal class caching which seems to temporarily expose method return values in some shared structure. The test case passes once CGLIB's caching is being disabled, and it also passes with a delay between those concurrent refreshes, indicating that the underlying CGLIB data structure is temporary in nature and is being cleaned up afterwards.

Note that removing the @Configuration marker from your class makes the test case pass as well. @Bean methods are being discovered on other classes too; they just don't get inter-@Bean method invocations semantics. As long as you're not invoking another @Bean method from within an @Bean method implementation, that might be a good enough solution for your scenario, avoiding the use of CGLIB altogether.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Turns out that this is caused by the static holding of CGLIB Callback objects in a generated Class. Since the class is being reused if the cache is active, derived from the very same original class in your context definitions, there is only set of callbacks statically registered; the latest one registered wins. Effectively, for concurrent instantiation of the same configuration class in different contexts, all of those instances are going to be linked to the ApplicationContext that came last in registering the static callbacks.

So with the present model, there isn't much we can do about this. We are certainly not going to deactivate CGLIB's class cache since that causes a perm gen leak so easily. Other than recommending to avoid concurrent creation of the same Configuration class in different contexts altogether, there's only the option to not make it a Configuration class in the first place but rather use simple @Bean methods (a.k.a. "configuration class lite"), as indicated above. Or we could provide a static setting to deactivate the class cache...

Juergen

[spring-projects-issues]

Lari Hotari commented

This change seems to cause a lot of blocking and contention in Spring 4 applications. In Grails 2.4, it seems to be causing most of the blocking.

examples of stack traces

http-bio-8080-exec-9 [BLOCKED] [DAEMON]
org.springframework.context.support.AbstractApplicationContext.assertBeanFactoryActive()
org.springframework.context.support.AbstractApplicationContext.getBean(String)
org.codehaus.groovy.grails.web.util.WebUtils.lookupApplication(ServletContext)
org.codehaus.groovy.grails.web.mapping.DefaultUrlMappingInfo.getMultipartDisabled()
org.codehaus.groovy.grails.web.mapping.DefaultUrlMappingInfo.tryMultipartParams(HttpServletRequest, Enumeration)
org.codehaus.groovy.grails.web.mapping.DefaultUrlMappingInfo.checkDispatchAction(HttpServletRequest)
org.codehaus.groovy.grails.web.mapping.DefaultUrlMappingInfo.getActionName()
org.codehaus.groovy.grails.web.mapping.UrlMappingUtils.getFeatureId(UrlConverter, UrlMappingInfo)
org.codehaus.groovy.grails.web.mapping.UrlMappingUtils.passControllerForUrlMappingInfoInRequest(GrailsWebRequest, UrlMappingInfo, UrlConverter, GrailsApplication)
org.codehaus.groovy.grails.web.mapping.filter.UrlMappingsFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.codehaus.groovy.grails.web.servlet.mvc.GrailsWebRequestFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.codehaus.groovy.grails.web.filters.HiddenHttpMethodFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(Filter, ServletRequest, ServletResponse, FilterChain)
org.springframework.web.filter.DelegatingFilterProxy.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.StandardWrapperValve.invoke(Request, Response)
org.apache.catalina.core.StandardContextValve.invoke(Request, Response)
org.apache.catalina.core.StandardHostValve.invoke(Request, Response)
org.apache.catalina.valves.ErrorReportValve.invoke(Request, Response)
org.apache.catalina.core.StandardEngineValve.invoke(Request, Response)
org.apache.catalina.connector.CoyoteAdapter.service(Request, Response)
org.apache.coyote.http11.AbstractHttp11Processor.process(SocketWrapper)
org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(SocketWrapper, SocketStatus)
org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run()
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor$Worker)
java.util.concurrent.ThreadPoolExecutor$Worker.run()
java.lang.Thread.run()

http-bio-8080-exec-13 [BLOCKED] [DAEMON]
org.springframework.context.support.AbstractApplicationContext.assertBeanFactoryActive()
org.springframework.context.support.AbstractApplicationContext.getBeansOfType(Class)
org.codehaus.groovy.grails.web.servlet.mvc.GrailsWebRequest.destroyPersistenceInterceptor()
org.codehaus.groovy.grails.web.servlet.mvc.GrailsWebRequest.requestCompleted()
org.codehaus.groovy.grails.web.servlet.GrailsDispatcherServlet.doDispatch(HttpServletRequest, HttpServletResponse)
org.springframework.web.servlet.DispatcherServlet.doService(HttpServletRequest, HttpServletResponse)
org.springframework.web.servlet.FrameworkServlet.processRequest(HttpServletRequest, HttpServletResponse)
org.springframework.web.servlet.FrameworkServlet.doGet(HttpServletRequest, HttpServletResponse)
javax.servlet.http.HttpServlet.service(HttpServletRequest, HttpServletResponse)
org.springframework.web.servlet.FrameworkServlet.service(HttpServletRequest, HttpServletResponse)
javax.servlet.http.HttpServlet.service(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.apache.tomcat.websocket.server.WsFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationDispatcher.invoke(ServletRequest, ServletResponse, ApplicationDispatcher$State)
org.apache.catalina.core.ApplicationDispatcher.processRequest(ServletRequest, ServletResponse, ApplicationDispatcher$State)
org.apache.catalina.core.ApplicationDispatcher.doForward(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationDispatcher.forward(ServletRequest, ServletResponse)
org.codehaus.groovy.grails.web.mapping.UrlMappingUtils.forwardRequestForUrlMappingInfo(HttpServletRequest, HttpServletResponse, UrlMappingInfo, Map, boolean)
org.codehaus.groovy.grails.web.mapping.UrlMappingUtils.forwardRequestForUrlMappingInfo(HttpServletRequest, HttpServletResponse, UrlMappingInfo, Map)
org.codehaus.groovy.grails.web.mapping.UrlMappingUtils.forwardRequestForUrlMappingInfo(HttpServletRequest, HttpServletResponse, UrlMappingInfo)
org.codehaus.groovy.grails.web.mapping.filter.UrlMappingsFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.codehaus.groovy.grails.web.servlet.mvc.GrailsWebRequestFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.codehaus.groovy.grails.web.filters.HiddenHttpMethodFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain)
org.springframework.web.filter.OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)
org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(Filter, ServletRequest, ServletResponse, FilterChain)
org.springframework.web.filter.DelegatingFilterProxy.doFilter(ServletRequest, ServletResponse, FilterChain)
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)
org.apache.catalina.core.StandardWrapperValve.invoke(Request, Response)
org.apache.catalina.core.StandardContextValve.invoke(Request, Response)
org.apache.catalina.core.StandardHostValve.invoke(Request, Response)
org.apache.catalina.valves.ErrorReportValve.invoke(Request, Response)
org.apache.catalina.core.StandardEngineValve.invoke(Request, Response)
org.apache.catalina.connector.CoyoteAdapter.service(Request, Response)
org.apache.coyote.http11.AbstractHttp11Processor.process(SocketWrapper)
org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(SocketWrapper, SocketStatus)
org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run()
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor$Worker)
java.util.concurrent.ThreadPoolExecutor$Worker.run()
java.lang.Thread.run()

[spring-projects-issues]

Juergen Hoeller commented

Lari, that assertBeanFactoryActive check is only really a defensive measure against external calls to the BeanFactory before context initialization or after context shutdown. Under which load conditions do you see that turning into a lock contention problem? I'm somewhat surprised since it's only really a guarded check against a boolean flag...

One way to optimize such heavily accessed code paths in Grails would be to delegate to the underlying BeanFactory directly: either through calling getBeanFactory() on the ConfigurableApplicationContext interface and performing the bean access on that raw BeanFactory, or through making your accessors BeanFactoryAware and using the given reference there (which is also the raw BeanFactory). It's generally advisable to use the most specific accessor pattern possible: even Provider<List<MyBean>> etc instead of repeated getBeansOfType(MyBean.class) calls for lazy access to all matching beans by type, etc.

Juergen

[spring-projects-issues]

Lari Hotari commented

@Juergen . I'm just relying on the profiler's (YourKit) information about blocking methods. This lock contention shows up in a simple Grails load test:
https://github.com/lhotari/grails-perf-testapps/tree/master/empty-test-app . There's scripts there to reproduce the problem. I'm using https://github.com/wg/wrk for generating load. wrk is much better than Apache Bench for load testing a single URL (it also works well on Mac OSX and doesn't run out of TCP/IP ports).

I usually do (Grails/Spring MVC) performance optimization by first minimizing blocking between request handling threads. I don't know the exact details of the JVM, but I assume that spin-locks are used for locks that aren't held for long time periods. Spin locks consume CPU time and eliminating that can improve performance. The differences are very small, but I think that all blocking should be prevented if that's possible. Lock-free request handling would make normal synchronous and blocking http request handling parallelizable and from Amdahl's law, we know that it's very important to eliminate all locks if possible (if we want to add performance by adding more cores to a single system). If request handling is lock free, it would be then only because of Little's law when we would be need to do asynchronous http request handling. I did a talk yesterday here at GR8Conf.EU about this and I was trying to explain this idea:
http://www.slideshare.net/lhotari/performance-tuning-grails-applications

Spring 3.2.x didn't have this problem and therefore I'm quite sure others will be reporting the same issue after doing some performance testing and profiling with Spring 4.

We started using getBeansOfType without our own caching solution after this was optimized in Spring:
#11536 , #13934 , #12604 , #14083, #12750

Talking about performance of Spring DI, we still have a custom solution in Grails to optimize autowiring. Grails autowires all objects that are retrieved from Hibernate (GORM) . That's why the performance of autowiring is really critical for Grails. That's using autowiring by name, so it doesn't relate to autowiring by type.
https://github.com/grails/grails-core/blob/master/grails-core/src/main/groovy/org/codehaus/groovy/grails/commons/spring/OptimizedAutowireCapableBeanFactory.java
It would be nice to have some caching like this directly in Spring.
Some usecases (I was perf. testing with) were 300% faster with this change at the time I added that optimization, so it's quite important for Grails applications.

So as conclusion, I would like to achieve lock-free request thread handling in Grails. That's my current strategy for keeping the Grails performance in good shape. The change in assertBeanFactoryActive method is currently breaking that and I'd like to get that fixed soon. As a temporary solution I can try to workaround the problem on Grails side, but I believe other Spring applications would also benefit of lock-free request thread handling.

Is it worth reporting this performance issue separately? It's not actually causing a big degrade in performance, but it's breaking the "lock-free request thread handling policy".

[spring-projects-issues]

Juergen Hoeller commented

Lari Hotari, that sounds like a fine goal. Let's handle those as enhancement requests for 4.1 then (any further caching within the factory), with the assertBeanFactoryActive lock improvement also to be backported to 4.0.6. Would be great to have separate JIRA issues for those...

As for the assertBeanFactoryActive lock affecting other kinds of Spring applications, it's worth noting that all injection/autowiring within the framework happens at the internal BeanFactory level, with the ApplicationContext facade not involved. That's probably why nobody else has reported that issue yet: It only really affects arrangements with heavy custom use of getBean/getBeansOfType on the ApplicationContext facade. That's what I meant above: Performing those same calls on the nested BeanFactory within the context would avoid the lock already.

Juergen

[spring-projects-issues]

Lari Hotari commented

@Juergen, thanks for the advice of using ConfigurableApplicationContext.getBeanFactory / BeanFactoryAware . We will have to do the changes in Grails to use the raw BeanFactory for bean lookups in performance critical code so that we get the speed up with Spring 4.0.x .

I'll report the 2 enhancement requests separately.

[spring-projects-issues]

Lari Hotari commented

I've filed #16482 as a feature request.

[spring-projects-issues]

Lari Hotari commented

I've filed #16483 for the autowiring performance optimizations.

[spring-projects-issues]

Lari Hotari commented

I also reported #16486 . The top blocker is now org.springframework.beans.CachedIntrospectionResults.forClass(Class) method in one my Grails performance tests which I have to optimize the overhead of Grails & Spring. This problem shows up after applying a workaround for this AbstactApplicationContext. assertBeanFactoryActive performance problem. It's probably been like this for a long time, but I just haven't reported it earlier.