Affects: 5.3.x
Hi,
AbstractBeanFactory implementation is causing issues in our application since version 5.3. The application randomly failed to initialize with the following error :
WARN o.s.b.w.s.c.AnnotationConfigServletWebServerApplicationContext - Exception encountered during context initialization - cancelling refresh attempt: org.springframework.context.ApplicationContextException: Unable to start web server; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'errorPageFilterRegistration' defined in org.springframework.boot.web.servlet.support.Error
PageFilterConfiguration: Unsatisfied dependency expressed through method 'errorPageFilterRegistration' parameter 0; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'errorPageFilter' defined in org.springframework.boot.web.servlet.support.ErrorPageFilterConfiguration: Initialization of bean failed; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.spring
framework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration': Instantiation of bean failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [org.springframework.boot.autoconfigu
re.web.servlet.error.ErrorMvcAutoConfiguration]: No default constructor found; nested exception is java.lang.NoSuchMethodException: org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration.<init>() []
I finally managed to understand what is happening and believe this can and should be fixed in spring-beans.
BeanPostProcessorCacheAwareList was introduced in AbstractBeanFactory in 5.3.
BeanPostProcessorCacheAwareList and accesses to the beanPostProcessors instance are not Thread safe. If multiple Threads are running during initialization and a Thread calls getBeanPostProcessorCache() while another Thread is calling addBeanPostProcessors, you can end up with a cache which does not contain all BeanPostProcessor instances and thus doesn't find the appropriate constructor.
Making BeanPostProcessorCacheAwareList and its usages in AbstractBeanFactory thread safe is relatively straightforward with synchronized blocks and would prevent such very obscure initialization issues.
Kind regards, Xavier
Comment From: xaviertesch
Hi @jhoeller, thanks for looking into this ! Considering the title update, I would simply like to note that the problem is not only in BeanPostProcessorCacheAwareList, but also in AbstractBeanFactory, as even with a fully thread safe BeanPostProcessorCacheAwareList, at least the two methods named addBeanPostProcessors(...) and addBeanPostProcessor(...) would still not be thread safe. At least unless you would group the remove + add calls in one method in BeanPostProcessorCacheAwareList.
Kind regards, Xavier
Comment From: jhoeller
Good catch. I'm aware of that remove/add part being the problem, we're going to see what we can do about it, maybe providing dedicated operations for those steps in BeanPostProcessorCacheAwareList indeed.
A general note: It is not common to perform post-processor registration concurrently. A typical Spring application startup is primarily single-threaded, at least during the post-processor registration phase.
Out of curiosity, what kind of concurrent startup arrangement are you using there?
Comment From: xaviertesch
On our side, this concurrent initialization was not on purpose. A Thread was started during the initialization phase, using a spring bean and triggering initialization of dependent beans in the Thread. All was working fine until 5.3 when we started to experience Exceptions in a small percentage of our application startups.
Clearly our code wasn't optimal but I thought there was no obvious reason not to make this possible in spring, especially considering the time lost on our side investigating this problem.
Note that we are considering concurrent initialization to improve startup times in the future, so if supported and useful, this might become critical for us again. In the meantime, I made sure not to use a different thread during the initialization phase.