Doc: DefaultListableBeanFactory is not thread-safe for manual singleton registration [SPR-12970]

[spring-projects-issues]

Mariano Gonzalez opened SPR-12970 and commented

Although the DefaultListableBeanFactory class uses ConcurrentHashMap to keep track of the contained objects, the manualSingletonNames is a simple LinkedHashMap.

As a result, if a Thread is manually registering a bean via the registerSingleton(String, Object) method, while another thread is executing the doGetBeanNamesForType(Class, boolean, boolean) -- or any of the other methods which iterate that Set -- then a ConcurrentModificationException is thrown.

It seems to me that this should be allowed and that if the maps are concurrent, then it makes sense that this Set is also.

Additionally I noticed that the beanDefinitionNames and resolvableDependencies attributes are also not synchronised. I haven't run into any issues with that, but it's still worth analysing.

Thank you!

---

Affects: 4.1.6

Issue Links:

• DefaultListableBeanFactory should allow efficient access to current bean names [SPR-12404] #17012 DefaultListableBeanFactory should allow efficient access to current bean names
• ConcurrentModificationException in DefaultListableBeanFactory.doGetBeanNamesForType() [SPR-13123] #17714 ConcurrentModificationException in DefaultListableBeanFactory.doGetBeanNamesForType()
• ConcurrentModificationException when executing AutowireCapableBeanFactory.createBean [SPR-13493] #18071 ConcurrentModificationException when executing AutowireCapableBeanFactory.createBean

Referenced from: commits 90493f4

[spring-projects-issues]

Juergen Hoeller commented

Singleton registration as well as resolvable dependency registration are supposed to be a configuration-time steps: that is, they should be stable once bean creation/access begins (typically as of the ApplicationContext refresh call). In contrast to that, the actual singletons instance map needs to be truly concurrent, since lazy singleton instances may get created from bean definitions at any time.

That said, we optimized the synchronization arrangement within DefaultListableBeanFactory as of 4.1.2, for efficient concurrent access with minimal locking. We'll revisit the iteration attempts for manual singletons and resolvable dependencies for 4.1.7, provided that our tradeoff can be kept intact, not compromising on runtime access performance for early-registration scenarios.

Juergen

[spring-projects-issues]

Mariano Gonzalez commented

Thank you Juergen!

[spring-projects-issues]

Juergen Hoeller commented

It turns out to be rather hard to allow for the late registration of new singleton beans without compromising on efficient registration handling and subsequently efficient autowiring. Also, the order of bean registration is significant in Spring, so we expect a well-defined order and have to preserve that order at runtime. A CopyOnWriteArraySet might technically do the job here but seems to be wasteful when registering a high number of beans...

Since late concurrent registration of beans was never officially intended in the first place, I'm inclined to leave it as-is and document it accordingly. Note that re-registering bean definitions or singleton instances for existing bean names should generally work fine: Such hot updates of existing beans are within the range of what we originally intended to support.

Juergen