micronaut-projects / micronaut-core

Micronaut Application Framework
http://micronaut.io
Apache License 2.0
6.03k stars 1.05k forks source link

Calls to @Client during constructor or PostConstruct of a Bean hang #3910

Open vijayender opened 4 years ago

vijayender commented 4 years ago

Thanks for reporting an issue, please review the task list below before submitting the issue. Your issue report will be closed if the issue is incomplete and the below tasks not completed.

NOTE: If you are unsure about something and the issue is more of a question a better place to ask questions is on Stack Overflow (https://stackoverflow.com/tags/micronaut) or Gitter (https://gitter.im/micronautfw/). DO NOT use the issue tracker to ask questions.

Task List

Steps to Reproduce

  1. Create a http Client that returns a json response
  2. Create a bean, and in the init{} or postConstruct section make the call via Client defined above
  3. After application bootup try accessing the bean in one of the calls. The application hangs.

Expected Behaviour

The Bean should get inited

Actual Behaviour

The call is hung My analysis:

Environment Information

Example Application

There is an example app to reproduce the issue:

Chordless commented 3 years ago

I ran into this same issue on the newest version of Micronaut (2.3.4). We have a Singleton bean, that in it's constructor does a HTTP call via a @Client class. When the HTTP call returns, some code in micronaut wants to parse the JSON response using Jackson, and calls into some code that tries to get the ObjectMapper singleton bean. This hangs until something times out.

What i think goes on:

  1. We're in the middle of some synchronized code in micronaut, that is new-ing up our singleton bean. The thread parks while waiting for a response to our HTTP call done in the constructor.
  2. The HTTP call returns on a netty thread, and while parsing the response it gets the ObjectMapper bean.
  3. This bean hasn't been used yet, so it needs to get new-ed up by Micronaut as a singleton bean.
  4. The code for creating singleton beans in micronaut is syncronized, so the netty thread parks while waiting for "whoever" is already newing up a singleton bean, to get finished.
  5. The two treads are now waiting on each other and we are deadlocked. Eventually some netty http client code times out and rejects the whole web-request as timed out.

Important points for repro: The http call done must be one that returns JSON. ObjectMapper must also not have been used anywhere else in the app before the HTTP call returns.

We managed to work around this by defining ObjectMapper as a @Context scoped bean in our project. If ObjectMapper already exists as a bean when the HTTP request returns, no deadlock happens.

yawkat commented 2 years ago

we need to improve error handling for nested bean initialization (i.e. throw an error)

yawkat commented 2 years ago

Ok I don't think we can detect such deadlocks reliably. Test case is this:

package io.micronaut.context

import jakarta.annotation.PostConstruct
import jakarta.inject.Inject
import jakarta.inject.Singleton
import spock.lang.Issue
import spock.lang.Specification
import spock.lang.Timeout

class InitializationDeadlockSpec extends Specification {
    @Timeout(10)
    @Issue('https://github.com/micronaut-projects/micronaut-core/issues/3910')
    def 'nested initialization on different threads should not lead to deadlock'() {
        given:
        def ctx = ApplicationContext.run()

        expect:
        ctx.getBean(BeanA) != null
        ctx.getBean(BeanB).visitedFromA
    }

    @Singleton
    static class BeanA {
        @Inject
        BeanContext context

        @PostConstruct
        void createB() {
            def thread = new Thread(() -> {
                context.getBean(BeanB).visitedFromA = true
            })
            thread.start()
            thread.join()
        }
    }

    @Singleton
    static class BeanB {
        boolean visitedFromA = false
    }
}

@graemerocher is it be feasible to reduce scope of locking in DefaultBeanContext so that this can work? I think that in principle it should be possible to lock only for the specific BeanKey that is being requested (instead of locking on the full singletonObjects), so that other threads can handle other BeanKeys concurrently. But not sure if there are other complications or how difficult this would be to build.

dstepanov commented 2 years ago

That could be fixed by doing synchronization on the instance of BeanDefinition in findOrCreateSingletonBeanRegistration but the problem is that we can have multiple instances of the same BeanDefinition. The other approach would be to have a map of locks...

graemerocher commented 2 years ago

@dstepanov when I looked at the CDI implementations out there that do this it appears a map of locks is what they do.

graemerocher commented 2 years ago

@dstepanov assigned to you, perhaps worth addressing as part of the broader refactor you are doing