https://github.com/spring-projects/spring-framework/blob/b1224835be7e591fc784c565a7efdb7f09d29d26/spring-context/src/main/java/org/springframework/context/support/AbstractRefreshableApplicationContext.java#L175

On large servers running Tomcat with lots of cores we see massive thread blocking in getBeanFactory() due to the synchronized block. This is hit as we're using XmlWebApplicationContext and any time a bean is looked up during a web request the bean factory is required.

Outside of web applications we're also having another context using GenericXmlApplicationContext which doesn't have that problem.

Would be great to get some insight to either find a workaround on our side or fix the implementation.

Thank you Axel

Comment From: jhoeller

Which code path typically hits the blocking there? Some form of getBean lookups on the WebApplicationContext reference, internally delegating to the BeanFactory every time? Our internal lookups often hold on to the inner BeanFactory and operate directly on it, this could also be a way out for custom retrieval code.

That said, we might be able to rework this (rather old) beanFactoryMonitor lock there with some finer-grained locking for read access, potentially even just as a volatile field for access and a dedicated lock for (re-)initialization and shutdown.

Comment From: axel-grossmann-sap

I'm just trying to collect some cases.

Here's an first interesting one which is a pure Spring one (TenantIgnoreXmlWebApplicationContext is our class but doesn't override containsBean() or anything in the way ). Seems that each request going through org.springframework.web.multipart.support.MultipartFilter is always doing a bean lookup and that's always hitting getBeanFactory().

...
TenantIgnoreXmlWebApplicationContext(AbstractRefreshableApplicationContext).getBeanFactory() line: 175  
    TenantIgnoreXmlWebApplicationContext(AbstractApplicationContext).containsBean(String) line: 1146    
    MultipartFilter.lookupMultipartResolver() line: 157 
    MultipartFilter.lookupMultipartResolver(HttpServletRequest) line: 143   
    MultipartFilter.doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain) line: 108    
    MultipartFilter(OncePerRequestFilter).doFilter(ServletRequest, ServletResponse, FilterChain) line: 119  
    ApplicationFilterChain.internalDoFilter(ServletRequest, ServletResponse) line: 193  
    ApplicationFilterChain.doFilter(ServletRequest, ServletResponse) line: 166  
    FilterChainProxy$VirtualFilterChain.doFilter(ServletRequest, ServletResponse) line: 320 
    FilterSecurityInterceptor.invoke(FilterInvocation) line: 127    
    FilterSecurityInterceptor.doFilter(ServletRequest, ServletResponse, FilterChain) line: 91

Comment From: jhoeller

Indeed, some of our filter implementations and also some web integration delegates (e.g. for JSF) call getBean methods at the WebApplicationContext level and therefore go through the lock.

While we could revise those spots to hold on to the nested BeanFactory instead, it seems more attractive to switch AbstractRefreshableApplicationContext to a volatile beanFactory field (which I tried locally already, seems to work fine so far). Since AbstractApplicationContext applies a startupShutdownMonitor already, we should be able to get rid of the beanFactoryMonitor completely, always accessing the volatile field instead.

Comment From: axel-grossmann-sap

I'm not sure where there is a nested BeanFactory, because in our stack traces there are always just those two two methods 'above' our own call to getBean(String): - org.springframework.context.support.AbstractApplicationContext.getBean(String, Class) calling - org.springframework.context.support.AbstractRefreshableApplicationContext.getBeanFactory()

May be we're missing out on something before that call, when we look up the current web application context the way below. (It's a static utility method we need for our legacy code base, just in case you wonder.) 

    private static ApplicationContext getWebApplicationContextIfExists()
    {
        ServletContext servletContext = null;

        final RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
        if (requestAttributes instanceof ServletRequestAttributes)
        {
            servletContext = ((ServletRequestAttributes) requestAttributes).getRequest().getServletContext();
        }

...
        if (servletContext != null)
        {
            final ApplicationContext context = WebApplicationContextUtils.getWebApplicationContext(servletContext);
            if (context == null)
            {
                if (log.isDebugEnabled())
                {
                    log.debug("No web spring configuration for webapp " + servletContext.getServletContextName()
                            + " found, using only core configuration.");
                }
            }
            else
            {
…
                return context;

            }
        }
        return null;
    }

Comment From: axel-grossmann-sap

Aside of that volatile sounds like a good way. 👍