Performance bottleneck and potential thread deadlock in DefaultSingletonBeanRegistry [SPR-8471] #13117
Assignees
Labels
has: votes-jira
Issues migrated from JIRA with more than 10 votes at the time of import
in: core
Issues in core modules (aop, beans, core, context, expression)
type: bug
A general bug
Milestone
Comments
|
ken lu commented We have the similiar issue when we are using Spring JMS MessageListener to specify <property name="concurrentConsumers" value="10"/> When Spring reads the context file, it starts 10 threads for the MessageListener. This creates a deadlock immediately when these threads get to the singleton Service or DAO. The problem is the Service and DAO singleton should not be instanced by these thread. The work around is to specify the MessageListener as lazy-init="true". Make sure all the singleton bean was created, then start the MessageListener. Really hope Spring framework should sort this type of multi-threaded issue in a better way! |
|
Jean-Pierre Bergamin commented Maybe the issue #13428 is related. |
|
John Cotter commented This appears to be root cause of #12374 and OSGI-816 also. DefaultSingletonBeanRegistry could be refined so as not to hold a global lock on all singletons. As it is now, a second thread calling into DefaultSingletonBeanRegistry originating from call to DefaultListableBeanFactory can yield a deadlock. There is a wide array of use cases where this can occur. |
|
John Cotter commented Similar deadlock pattern involving DefaultSingletonBeanRegistry and DefaultListableBeanFactory observed with Spring 3.0.5. |
|
Chris Beams commented Linking as possibly related to #12374 per John's comment. |
|
Piotr Bzdyl commented I encountered a similar issue but it involved:
Deadlock report: |
|
Piotr Bzdyl commented I forgot to mention the deadlock occurred in 3.0.5.RELEASE. |
|
Alex Rau commented Same here with 3.1.0.RELEASE |
|
Nicholas DiPiazza commented This test case reproduces the issue. |
|
Chris Beams commented Thanks, Nicholas. |
|
Juergen Hoeller commented Note that along with the changes for #14452, several locking improvements made it into Spring Framework 3.2, including changes that avoid the deadlock potential outlined in the comments on this issue. The deadlock-related changes have also been backported to Spring Framework 3.1.4. Juergen |
|
Hendy Irawan commented I'm still getting this issue using 3.2.8.RELEASE : |
|
Hendy Irawan commented Blocks soluvas/soluvas-framework#66 |
|
Yvonnick Esnault commented Hi, This issue is not resolved (version 3.2.5 here) "HTTP-20" prio=10 tid=0x00007f673408b000 nid=0x7f0f waiting for monitor entry [0x00007f68c9761000]
java.lang.Thread.State: BLOCKED (on object monitor)
at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:181)
- waiting to lock <0x00000007683f4fc8> (a java.util.concurrent.ConcurrentHashMap)
at org.springframework.beans.factory.support.AbstractBeanFactory.isSingleton(AbstractBeanFactory.java:386)
at org.springframework.beans.factory.support.DefaultListableBeanFactory.doGetBeanNamesForType(DefaultListableBeanFactory.java:356)
at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBeanNamesForType(DefaultListableBeanFactory.java:326)
at org.springframework.context.support.AbstractApplicationContext.getBeanNamesForType(AbstractApplicationContext.java:1178) |
|
Juergen Hoeller commented Henry, Yvonnick, Could you please clarify: Are we talking about thread contention here? Specifically, it doesn't seem like you're running into a deadlock but rather 'just' into wait times for a lock? Juergen |
|
Yvonnick Esnault commented It's a deadlock : with schedulerFactory_Worker-7 like that : and HTTP-20 : |
|
Juergen Hoeller commented Hmm that seems to be a deadlock between Spring's singleton lock in the bean factory and some related lock behind a CXF FactoryBean... I'm afraid it's no good idea to wrap Spring bean lookup calls in a custom lock there in CXF, when CXF is currently within a Spring-driven bean creation attempt itself! Juergen |
|
Yvonnick Esnault commented Some information from the CXF mailing list here http://mail-archives.apache.org/mod_mbox/cxf-users/201311.mbox/%[email protected]%3E |
|
Juergen Hoeller commented That sounds like they'll relax their custom lock in CXF which is definitely worthwhile. I'm afraid we can't relax the singleton lock on our side, otherwise we'd immediately expose ourselves to a deadlock potential during the creation of lazy singletons with dependencies on other lazy singletons... So I wonder: What specifically could we do on our side still, while keeping our factory-wide singleton lock? Juergen |
|
Juergen Hoeller commented I'll keep this marked as resolved for 3.2 then, since I don't see any further changes that we could make without sacrificing our singleton guarantees... Juergen |
spring-projects-issues
added
type: bug
This was referenced Jan 11, 2019
This was referenced Jan 11, 2019
|
This is still an issue in 2019. |
|
I've encountered the same issue with Spring Boot 2.3.1.RELEASE / Spring 5.2.7.RELEASE... "Thread-6" #48 prio=5 os_prio=0 cpu=22.27ms elapsed=500.34s tid=0x00007f8642ffa000 nid=0x402e waiting for monitor entry [0x00007f8528559000]
java.lang.Thread.State: BLOCKED (on object monitor)
at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:179)
- waiting to lock <0x00000000c01f1320> (a java.util.concurrent.ConcurrentHashMap)
at org.springframework.beans.factory.support.AbstractBeanFactory.isTypeMatch(AbstractBeanFactory.java:517)
at org.springframework.beans.factory.support.DefaultListableBeanFactory.doGetBeanNamesForType(DefaultListableBeanFactory.java:527)
at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBeanNamesForType(DefaultListableBeanFactory.java:499)
at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBeanNamesForType(DefaultListableBeanFactory.java:485)
at org.springframework.beans.factory.BeanFactoryUtils.beanNamesForTypeIncludingAncestors(BeanFactoryUtils.java:228)
at org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils.qualifiedBeanOfType(BeanFactoryAnnotationUtils.java:118)
at org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils.qualifiedBeanOfType(BeanFactoryAnnotationUtils.java:95)
at org.springframework.transaction.interceptor.TransactionAspectSupport.determineQualifiedTransactionManager(TransactionAspectSupport.java:492)
at org.springframework.transaction.interceptor.TransactionAspectSupport.determineTransactionManager(TransactionAspectSupport.java:473)
at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:335)
at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:99)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:139)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor$CrudMethodMetadataPopulatingMethodInterceptor.invoke(CrudMethodMetadataPostProcessor.java:178)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:95)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:212)
at com.sun.proxy.$Proxy165.findAll(Unknown Source)
at my.package.Abc.lambda$initialize$0(Abc.java:34)
at my.package.Abc$$Lambda$919/0x00000008409c0440.run(Unknown Source)
at java.lang.Thread.run(java.base@11.0.7/Thread.java:834) |
|
@jhoeller -- I have encountered same issue with Spring release 5.2.1. Please suggest what to do to. Also, please reopen this issue if possible. Thanks |
|
@CODESTHN Are you seeing specific thread contention for |
|
Reviewing the recent stacktraces above, we seem to have a specific case about I'll create a GitHub issue myself to that effect, even for 5.2.9 since such a change seems straightforward enough. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Mark Davies opened SPR-8471 and commented
A less significant version of this problem has already been raised under #10033 - a performance bottleneck affecting Wicket. However, the same issue causes a serious thread deadlock in our application, occasionally preventing application startup.
The basic issue seems to be that DefaultSingletonBeanRegistry takes a global synchronization lock when creating a singleton bean. Here is the code in that class:
The synchronized block (the same this.singletonObjects reference is used by all threads entering the method) means that Spring can only create singleton beans one at a time, regardless of their type (beanName). This clearly introduces a performance penalty if an application has a large number of singleton beans to construct, e.g. at startup.
That is not the issue affecting our code, though. We see a deadlock, caused by the following two sets of behaviour:
We have Spring-managed singleton beans which perform database access in their constructor (basically loading and caching configuration sets from the database). In order to do this they obtain database connections, which are pooled, with relatively small pool sizes. If a pooled connection is not available, the calling thread blocks and waits until one becomes free. This is usually not a problem since queries are small and rapid, so pool wait times are low, and the maximum pool size is sufficient to work the databasea at full capacity anyway.
We also have non-Spring code doing database access. Such code obtains a database connection from the same pool, purely for the lifetime of running a query and processing a result set, so again very quick for almost cases in our system. But sometimes, whilst processing the result set, we need to use a Spring-managed bean, which may have singleton scope.
You now have a deadlock - thread number one is trying to get a Spring-managed singleton bean, which is waiting for a JDBC connection in its constructor; thread number two is running database code which has the JDBC connection and is waiting to create a Spring-managed singleton of a completely different type. Both thread own the resource needed by the other, so will wait forever. (OK, if the database pool size is two or higher you need two or more threads in the second state, but this has happened to us in customer environments, in production).
Obviously our application code is almost certainly less than ideal in how we use Spring, but it seems to me that we ought to be able to use Spring-managed beans which do database access in the manner I've described, without encountering unpleasant deadlocks such as this. Note that the problem is actually much more general than this particular example of database connections: if your singleton beans require any kind of global monitor, which is also used outside of the context of Spring, you have a deadlock condition.
Both the originally-reported performance problem, and this deadlock, could be solved by a simple improvement to DefaultSingletonBeanRegistry. The getSingleton() method should not synchronize on a global monitor, but instead a monitor specific to the beanName you are instantiating. This is the correct level at which to lock - what you are trying to do is prevent a bean of a certain type being created more than once, but allowing two beans of different types to construct at the same time is perfectly reasonable. Of course you have to synchronize access to the underlying collections with a global lock, but the construction of the singleton bean does not need to be protected with the same global monitor. For instance I believe this kind of thing would do it:
It would be extremely helpful if this approach could be implemented in this core part of the Spring framework. We rely on singleton construction to be performant and thread-safe; the fact that DefaultSingletonBeanRegistry behaves as it currently does is causing us serious production problems at client sites, and, whilst we can modify our application code to work around the issue, I do see this as a fundamental flaw in Spring - would be nice to get a quick fix please.
Affects: 2.5.6
Attachments:
Issue Links:
Referenced from: commits dd68fec, 52124fa
19 votes, 26 watchers
The text was updated successfully, but these errors were encountered: