Address race condition within spring that causes about-to-be-created-bean exceptions [SPR-16627]

[spring-projects-issues]

Brian Toal opened SPR-16627 and commented

Typically DefaultListableBeanFactory.doGetBeanNamesForType is triggered via ApplicationContext.refresh via EventListenerMethodProcessor.getEventListnerFactories() in a single thread prior to when the application context is available for use. See typical stack:

ConstructorResolver.instantiateUsingFactoryMethod(String, RootBeanDefinition, Object[]) line: 355
DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).instantiateUsingFactoryMethod(String, RootBeanDefinition, Object[]) line: 1250
DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).createBeanInstance(String, RootBeanDefinition, Object[]) line: 1099
DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).getSingletonFactoryBeanForTypeCheck(String, RootBeanDefinition) line: 946
DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).getTypeForFactoryBean(String, RootBeanDefinition) line: 833
DefaultListableBeanFactory(AbstractBeanFactory).isTypeMatch(String, ResolvableType) line: 557
DefaultListableBeanFactory.doGetBeanNamesForType(ResolvableType, boolean, boolean) line: 428
DefaultListableBeanFactory.getBeanNamesForType(Class, boolean, boolean) line: 399 DefaultListableBeanFactory.getBeanNamesForType(Class) line: 385
AnnotationConfigApplicationContext(AbstractApplicationContext).getBeanNamesForType(Class<?>) line: 1182

However in application thats setup doesn't cause DefaultListableBeanFactory.doGetBeanNamesForType to run prior to AC.refresh finishing, then there is thread safety issue when two threads race calling DefaultListableBeanFactory.getBeanNamesForType

t0 T1 AbstractBeanFactory.isTypeMatch checks for bean via getSingleton which returns null
t1 T2 AbstractBeanFactory.isTypeMatch checks for bean via getSingleton which returns null
t2 T1 AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck acquires getSingletonMutex lock
t3 T2 AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck blocks on getSingletonMutex lock
t4 T1 Creates bean instance by callingCreateBeanInstance all the way down to
ConstructorResovler.instantiateUsingFactoryMethod which calls getBean that
creates the non existing instance and puts it into the registry.
t5 T1 Releases getSingletonMutex lock
t6 T2 Acquires lock and attempts to create factory bean instance, however when
runtime gets to instantiateUsingFactoryMethod and the beanFactory.containsSingleton
occurs to check if the factory bean has been added to the bean factory, which it was
in T4, which causes the ImplicitlyAppeardSingletonException to be thrown.

To prevent this from happening the proposed change is to always have AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck check after it acquires the mutex if the singleton factory bean has been added in the registry, guarding it from failing with ImplicitlyAppeardSingletonException when another thread added the factory bean to the registry sometime between t1 and t5.

See the following project of an example of how this issue is reproduced:

https://github.com/toaler/spring-race-issue

Specifically:

https://github.com/toaler/spring-race-issue/blob/master/src/test/java/sandbox/SpringCreateFactoryBeanSingletonRaceTest.java

A potential fix for this issue has been submitted here. If you flip the spring version using in the maven pom from existing version, to the version built from the PR, you'll see the about-to-be-created-bean exception without the fix, and normal behavior with the fix.

#1750

---

Affects: 4.3.14, 5.0.3

Issue Links:

• FactoryBeanRegistrySupport atomicity issues [SPR-16625] #21166 FactoryBeanRegistrySupport atomicity issues

Referenced from: pull request #1750

Backported to: 4.3.15

[spring-projects-issues]

Brian Toal commented

ISTM that this issue and #21166 are thread safety issues in different paths.

[spring-projects-issues]

Juergen Hoeller commented

Indeed, and in related classes so I'm actually resolving both of them together, using a variant of the change in your pull request (just with an additional instanceof FactoryBean for defensiveness)... to be included in next week's 5.0.5 and 4.3.15 releases. Thanks for raising this!