Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async advisor retrieval blocks when triggered by singleton init method [SPR-14324] #18896

Closed
spring-projects-issues opened this issue Jun 1, 2016 · 8 comments

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jun 1, 2016

terry.zhu opened SPR-14324 and commented

in spring 4.2.6
class: LazySingletonAspectInstanceFactoryDecorator
method:getAspectInstance

thread is blocked.

please see:https://github.com/zhuyijian135757/http-server.git

run: class Booter
run test: class HttpClientCase


Affects: 3.2.17, 4.2.6, 4.3 GA

Reference URL: https://github.com/zhuyijian135757/http-server.git

Attachments:

Issue Links:

  • #18814 Deadlock possible with AspectJ aspects and multi-threading
  • #18961 Deadlock while creating a new thread on bean initialization with transactional code invocation

Backported to: 4.2.9, 3.2.18

1 votes, 4 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jun 1, 2016

Juergen Hoeller commented

Could you elaborate a bit? What behavior are you seeing? That synchronization change in 4.2.6, driven by #18814, was meant to avoid a deadlock between the local lock here and the context-wide singleton lock. This may lead to longer lock times for some calls which should be acceptable given that it avoids deadlocks.

Or are you saying that you're seeing a deadlock now against 4.2.6 which you haven't had against 4.2.5?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jun 15, 2016

Chris Klinger commented

It seems that we have a similar issue with our application here, but i wasn't able to reproduce it in a "simple" test case yet.

The scenario is: The application context is populated over several configurations, one of them is responsible for building up a elastic search index if not existing in @PostConstruct method. This method should block until the index is "ready" so we wait for future results.
The index is build up with @Async methods. Threadpool and everything is starting, but as soon the threads are starting to work they switch from "Running" to "Blocking" the moment they try to call a database service (which is a spring component ... @Transactional and own aspects are involved)

My analysis shows: The Main thread starting up the application context owns the org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.singletonObjects which is blocking the async threads from running in org.springframework.aop.aspectj.annotation.LazySingletonAspectInstanceFactoryDecorator.getAspectInstance line: 47 on the call of the database service.

The application is starting with 4.2.5 but not with 4.2.6. Additional observation we can't explain yet is that the application is starting up in 4.2.5 with single core configuration (that uses context scan to find additional ones) but not in an integration test using Spring JUnit Runner. In this case i additionally have to pass the Configuration class with the ThreadPoolTaskExecutor to get the test running, otherwise it's already deadlocking in 4.2.5

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 20, 2016

Efi Cohen commented

We are seeing a very similar problem after upgrading from 4.2.4

Application threads are stuck in BLOCKED state and the application won't start (seems like a deadlock as it never gets out from the blocked state)

java.lang.Thread.State: BLOCKED
at org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor.getAdvice(AbstractBeanFactoryPointcutAdvisor.java:101)

  • waiting to lock <44cac3e8> (a java.util.concurrent.ConcurrentHashMap) owned by "main" t@1
    at org.springframework.aop.framework.adapter.DefaultAdvisorAdapterRegistry.getInterceptors(DefaultAdvisorAdapterRegistry.java:81)
    at org.springframework.aop.framework.DefaultAdvisorChainFactory.getInterceptorsAndDynamicInterceptionAdvice(DefaultAdvisorChainFactory.java:65)
    at org.springframework.aop.framework.AdvisedSupport.getInterceptorsAndDynamicInterceptionAdvice(AdvisedSupport.java:489)
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:641)

It's happening every time we try and start the app with the new spring version

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 20, 2016

Juergen Hoeller commented

Efi Cohen, do you have a thread dump for the thread that's holding the lock ("main") as well? Which thread is triggering that advisor resolution step here, and which code initiated that thread to begin with? Is that code waiting for the other thread to finish, causing the deadlock that way?

In other words, do you happen to have any singleton initialization code that delegates out to an asynchronous task, waiting for it to return before completing the singleton's own initialization... and therefore not releasing the singleton lock before the other thread finishes?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 20, 2016

Juergen Hoeller commented

Generally speaking, it's a highly fragile arrangement to wait for an asynchronous task from an init method (@PostConstruct or any other method called within the singleton lock). Even if LazySingletonAspectInstanceFactoryDecorator wouldn't obtain the singleton lock there, the underlying getBean call would obtain it when accessing a singleton which hasn't been created yet...

For a possible refinement, we could leniently retrieve existing singleton instances like we did before, only running into the actual singleton lock for a newly created bean. I'm giving this a try for 4.3.4, to be backported to 4.2.9.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 20, 2016

Efi Cohen commented

@juergen.hoeller main thread dump attached.

The main is executing a synchronous code in a bean's post construct

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 20, 2016

Juergen Hoeller commented

Alright, that's analogous to the scenario outlined by Chris Klinger above then: an init method waiting for an async task to complete, with that async task having an aspect/advisor that refers back to a singleton bean in the factory. Just to be clear, this would not have worked before either if that singleton bean didn't exist yet... but point taken, the changes behind #18961 (your particular case) or #18814 (Chris Klinger's case) make it block for existing singleton beans as well now. I'll revise this for 4.3.4 and 4.2.9, avoiding an explicit lock for existing instances... but I highly recommend a revision on your side where your @PostConstruct method does not have to wait for the outcome of that async task to begin with.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 20, 2016

Efi Cohen commented

Thanks, much appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants