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

Race condition on concurrent ApplicationContext start [3.10.x] #10091

Closed
PakhomovAlexander opened this issue Nov 9, 2023 · 4 comments · Fixed by #10143
Closed

Race condition on concurrent ApplicationContext start [3.10.x] #10091

PakhomovAlexander opened this issue Nov 9, 2023 · 4 comments · Fixed by #10143
Assignees
Labels
status: validated An issue that has been validated as being a bug

Comments

@PakhomovAlexander
Copy link

PakhomovAlexander commented Nov 9, 2023

Expected Behavior

Concurrent calls for Micronaut#start() will launch multiple servers without errors.

Actual Behaviour

The code in Steps To Reproduce part sometimes fails with the output into stderr:

io.micronaut.context.exceptions.DependencyInjectionException: Failed to inject value for parameter [enabled] of method [setEnabled] of class: io.micronaut.http.server.HttpServerConfiguration$CorsConfiguration

Message: Failed to inject value for parameter [enabled] of method [setEnabled] of class: io.micronaut.http.server.HttpServerConfiguration$CorsConfiguration

Message: Error resolving property value [micronaut.server.cors.enabled]. Property doesn't exist
Path Taken: NettyEmbeddedServer.buildDefaultServer(NettyHttpServerConfiguration configuration) --> new DefaultNettyEmbeddedServerFactory(ApplicationContext applicationContext,[RouteExecutor routeExecutor],MediaTypeCodecRegistry mediaTypeCodecRegistry,StaticResourceResolver staticResourceResolver,ThreadFactory nettyThreadFactory,HttpCompressionStrategy httpCompressionStrategy,EventLoopGroupFactory eventLoopGroupFactory,EventLoopGroupRegistry eventLoopGroupRegistry) --> new RouteExecutor([Router router],BeanContext beanContext,RequestArgumentSatisfier requestArgumentSatisfier,HttpServerConfiguration serverConfiguration,ErrorResponseProcessor errorResponseProcessor,ExecutorSelector executorSelector) --> new DefaultRouter([Collection builders]) --> new AnnotatedFilterRouteBuilder(BeanContext beanContext,ExecutionHandleLocator executionHandleLocator,UriNamingStrategy uriNamingStrategy,ConversionService conversionService,[ServerContextPathProvider contextPathProvider]) --> NettyHttpServerConfiguration.setCors([CorsConfiguration cors]) --> CorsConfiguration.setEnabled([boolean enabled])
Path Taken: NettyEmbeddedServer.buildDefaultServer(NettyHttpServerConfiguration configuration) --> new DefaultNettyEmbeddedServerFactory(ApplicationContext applicationContext,[RouteExecutor routeExecutor],MediaTypeCodecRegistry mediaTypeCodecRegistry,StaticResourceResolver staticResourceResolver,ThreadFactory nettyThreadFactory,HttpCompressionStrategy httpCompressionStrategy,EventLoopGroupFactory eventLoopGroupFactory,EventLoopGroupRegistry eventLoopGroupRegistry) --> new RouteExecutor([Router router],BeanContext beanContext,RequestArgumentSatisfier requestArgumentSatisfier,HttpServerConfiguration serverConfiguration,ErrorResponseProcessor errorResponseProcessor,ExecutorSelector executorSelector) --> new DefaultRouter([Collection builders]) --> new AnnotatedFilterRouteBuilder(BeanContext beanContext,ExecutionHandleLocator executionHandleLocator,UriNamingStrategy uriNamingStrategy,ConversionService conversionService,[ServerContextPathProvider contextPathProvider]) --> NettyHttpServerConfiguration.setCors([CorsConfiguration cors]) --> CorsConfiguration.setEnabled([boolean enabled])
	at io.micronaut.context.exceptions.DependencyInjectionException.missingProperty(DependencyInjectionException.java:289)
	at io.micronaut.context.AbstractInitializableBeanDefinition.lambda$resolvePropertyValue$10(AbstractInitializableBeanDefinition.java:2043)
	at java.base/java.util.Optional.orElseThrow(Optional.java:408)
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolvePropertyValue(AbstractInitializableBeanDefinition.java:2043)
	at io.micronaut.context.AbstractInitializableBeanDefinition.getPropertyValueForSetter(AbstractInitializableBeanDefinition.java:1086)
	at io.micronaut.http.server.$HttpServerConfiguration$CorsConfiguration$Definition.injectBean(Unknown Source)
	at io.micronaut.http.server.$HttpServerConfiguration$CorsConfiguration$Definition.build(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.resolveByBeanFactory(DefaultBeanContext.java:2354)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2305)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2251)
	at io.micronaut.context.DefaultBeanContext.createRegistration(DefaultBeanContext.java:3016)
	at io.micronaut.context.SingletonScope.getOrCreate(SingletonScope.java:80)
	at io.micronaut.context.DefaultBeanContext.findOrCreateSingletonBeanRegistration(DefaultBeanContext.java:2918)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2879)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2800)
	at io.micronaut.context.DefaultBeanContext.getBean(DefaultBeanContext.java:1617)
	at io.micronaut.context.AbstractBeanResolutionContext.getBean(AbstractBeanResolutionContext.java:66)
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolveBean(AbstractInitializableBeanDefinition.java:2065)
	at io.micronaut.context.AbstractInitializableBeanDefinition.getBeanForSetter(AbstractInitializableBeanDefinition.java:1200)
	at io.micronaut.http.server.netty.configuration.$NettyHttpServerConfiguration$Definition.injectBean(Unknown Source)
	at io.micronaut.http.server.netty.configuration.$NettyHttpServerConfiguration$Definition.build(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.resolveByBeanFactory(DefaultBeanContext.java:2354)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2305)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2251)
	at io.micronaut.context.DefaultBeanContext.createRegistration(DefaultBeanContext.java:3016)
	at io.micronaut.context.SingletonScope.getOrCreate(SingletonScope.java:80)
	at io.micronaut.context.DefaultBeanContext.findOrCreateSingletonBeanRegistration(DefaultBeanContext.java:2918)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2879)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2800)
	at io.micronaut.context.DefaultBeanContext.getBean(DefaultBeanContext.java:1617)
	at io.micronaut.context.AbstractBeanResolutionContext.getBean(AbstractBeanResolutionContext.java:66)
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolveBean(AbstractInitializableBeanDefinition.java:2065)
	at io.micronaut.context.AbstractInitializableBeanDefinition.getBeanForConstructorArgument(AbstractInitializableBeanDefinition.java:1297)
	at io.micronaut.web.router.$AnnotatedFilterRouteBuilder$Definition.build(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.resolveByBeanFactory(DefaultBeanContext.java:2354)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2305)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2251)
	at io.micronaut.context.DefaultBeanContext.createRegistration(DefaultBeanContext.java:3016)
	at io.micronaut.context.SingletonScope.getOrCreate(SingletonScope.java:80)
	at io.micronaut.context.DefaultBeanContext.findOrCreateSingletonBeanRegistration(DefaultBeanContext.java:2918)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2879)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2853)
	at io.micronaut.context.DefaultBeanContext.addCandidateToList(DefaultBeanContext.java:3511)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistrations(DefaultBeanContext.java:3457)
	at io.micronaut.context.DefaultBeanContext.getBeanRegistrations(DefaultBeanContext.java:3427)
	at io.micronaut.context.DefaultBeanContext.getBeansOfType(DefaultBeanContext.java:1381)
	at io.micronaut.context.AbstractBeanResolutionContext.getBeansOfType(AbstractBeanResolutionContext.java:72)
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolveBeansOfType(AbstractInitializableBeanDefinition.java:2161)
	at io.micronaut.context.AbstractInitializableBeanDefinition.getBeansOfTypeForConstructorArgument(AbstractInitializableBeanDefinition.java:1437)
	at io.micronaut.web.router.$DefaultRouter$Definition.build(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.resolveByBeanFactory(DefaultBeanContext.java:2354)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2305)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2251)
	at io.micronaut.context.DefaultBeanContext.createRegistration(DefaultBeanContext.java:3016)
	at io.micronaut.context.SingletonScope.getOrCreate(SingletonScope.java:80)
	at io.micronaut.context.DefaultBeanContext.findOrCreateSingletonBeanRegistration(DefaultBeanContext.java:2918)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2879)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2800)
	at io.micronaut.context.DefaultBeanContext.getBean(DefaultBeanContext.java:1617)
	at io.micronaut.context.AbstractBeanResolutionContext.getBean(AbstractBeanResolutionContext.java:66)
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolveBean(AbstractInitializableBeanDefinition.java:2065)
	at io.micronaut.context.AbstractInitializableBeanDefinition.getBeanForConstructorArgument(AbstractInitializableBeanDefinition.java:1297)
	at io.micronaut.http.server.$RouteExecutor$Definition.build(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.resolveByBeanFactory(DefaultBeanContext.java:2354)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2305)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2251)
	at io.micronaut.context.DefaultBeanContext.createRegistration(DefaultBeanContext.java:3016)
	at io.micronaut.context.SingletonScope.getOrCreate(SingletonScope.java:80)
	at io.micronaut.context.DefaultBeanContext.findOrCreateSingletonBeanRegistration(DefaultBeanContext.java:2918)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2879)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2800)
	at io.micronaut.context.DefaultBeanContext.getBean(DefaultBeanContext.java:1617)
	at io.micronaut.context.AbstractBeanResolutionContext.getBean(AbstractBeanResolutionContext.java:66)
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolveBean(AbstractInitializableBeanDefinition.java:2065)
	at io.micronaut.context.AbstractInitializableBeanDefinition.getBeanForConstructorArgument(AbstractInitializableBeanDefinition.java:1297)
	at io.micronaut.http.server.netty.$DefaultNettyEmbeddedServerFactory$Definition.build(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.resolveByBeanFactory(DefaultBeanContext.java:2354)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2305)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2251)
	at io.micronaut.context.DefaultBeanContext.createRegistration(DefaultBeanContext.java:3016)
	at io.micronaut.context.SingletonScope.getOrCreate(SingletonScope.java:80)
	at io.micronaut.context.DefaultBeanContext.findOrCreateSingletonBeanRegistration(DefaultBeanContext.java:2918)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2879)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2800)
	at io.micronaut.context.DefaultBeanContext.getBean(DefaultBeanContext.java:1617)
	at io.micronaut.context.DefaultBeanContext.getBean(DefaultBeanContext.java:1599)
	at io.micronaut.http.server.netty.$DefaultNettyEmbeddedServerFactory$BuildDefaultServer0$Definition.build(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.resolveByBeanFactory(DefaultBeanContext.java:2354)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2305)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2251)
	at io.micronaut.context.DefaultBeanContext.createRegistration(DefaultBeanContext.java:3016)
	at io.micronaut.context.SingletonScope.getOrCreate(SingletonScope.java:80)
	at io.micronaut.context.DefaultBeanContext.findOrCreateSingletonBeanRegistration(DefaultBeanContext.java:2918)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2879)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2800)
	at io.micronaut.context.DefaultBeanContext.findBean(DefaultBeanContext.java:1680)
	at io.micronaut.context.DefaultBeanContext.findBean(DefaultBeanContext.java:1655)
	at io.micronaut.context.DefaultBeanContext.findBean(DefaultBeanContext.java:878)
	at io.micronaut.context.BeanLocator.findBean(BeanLocator.java:291)
	at io.micronaut.runtime.Micronaut.start(Micronaut.java:77)

Steps To Reproduce

 static Micronaut buildMicronautContext(int portCandidate) {
        Micronaut micronaut = Micronaut.build("");

        Map<String, Object> properties = new HashMap<>();
        properties.putAll(serverProperties(portCandidate));

        return micronaut
                .properties(properties)
                .banner(false)
                .mapError(DependencyInjectionException.class, ex -> { // This is the actual exception that is thrown
                    System.out.println("Got exception: " + ex);
                    ex.printStackTrace();
                    return 111;
                });
    }

    static Map<String, Object> serverProperties(int port) {
        Map<String, Object> result = new HashMap<>();
        result.put("micronaut.server.port", port);
        result.put("micronaut.server.cors.enabled", "true");
        result.put("micronaut.server.cors.configurations.web.allowed-headers", "Authorization");

        return result;
    }

    public static void main(String[] args) throws ExecutionException, InterruptedException {
        AtomicInteger portShift = new AtomicInteger(0);
        ExecutorService executorService = Executors.newFixedThreadPool(16);
        CopyOnWriteArrayList<Future<ApplicationContext>> futures = new CopyOnWriteArrayList<>();

        for (int i = 0; i < 50; i++) {
            var f = executorService.submit(() -> {
                try {
                    int port = 10300 + portShift.getAndIncrement();
                    var micronaut = buildMicronautContext(port);
                    System.out.println("Starting server on: " + port);
                    return micronaut.start();
                } catch (Throwable th) {
                    System.out.println("Failed to start: " + th.getMessage());
                    return null;
                }
            });
            futures.add(f);
        }

        ArrayList<ApplicationContext> contexts = new ArrayList<>();
        for (Future<ApplicationContext> future : futures) {
            try {
                ApplicationContext ctx = future.get();
                contexts.add(ctx);
                System.out.println("Started on: " + ctx.getEnvironment().getProperty("micronaut.server.port", Integer.class).orElse(-1));
            } catch (Throwable th) {
                System.out.println("Failed to start 2: " + th.getMessage());
            }
        }

        for (ApplicationContext context : contexts) {
            try {
                context.stop();
            } catch (Throwable th) {
                System.out.println("Failed to stop: " + th.getMessage());
            }
        }
        executorService.shutdown();
    }

Environment Information

  • JDK version java version "17.0.8" 2023-07-18 LTS

Example Application

No response

Version

3.10.x

@PakhomovAlexander
Copy link
Author

PakhomovAlexander commented Nov 9, 2023

Workaround

Call micronaut.start(); under the global lock.

The root cause

DefaultConversionService.line:143

ConvertiblePair pair = new ConvertiblePair(sourceType, targetType, null);
             // Some thread gets null for 'micronaut.server.cors.enabled' because converterCache has been reset by other thread
            TypeConverter typeConverter = converterCache.get(pair); // line 144
            if (typeConverter == null) {
                // Also gets null here 
                typeConverter = findTypeConverter(sourceType, targetType, null);
                if (typeConverter == null) {
                    // Puts UNCONVERTIBLE into the converter cache 
                    // So, another thread reads this value from the cache on the line 144 
                    // This causes the error 
                    converterCache.put(pair, UNCONVERTIBLE); 
                    return Optional.empty();
                } else {
                    converterCache.put(pair, typeConverter);
                    if (typeConverter == UNCONVERTIBLE) {
                        return Optional.empty();
                    } else {
                        return typeConverter.convert(object, targetType, context);
                    }
                }

Here is how findTypeConverter returns null:

 protected <T> TypeConverter findTypeConverter(Class<?> sourceType, Class<T> targetType, String formattingAnnotation) {
        TypeConverter typeConverter = UNCONVERTIBLE;
        List<Class> sourceHierarchy = ClassUtils.resolveHierarchy(sourceType);
        List<Class> targetHierarchy = ClassUtils.resolveHierarchy(targetType);
        for (Class sourceSuperType : sourceHierarchy) {
            for (Class targetSuperType : targetHierarchy) {
                ConvertiblePair pair = new ConvertiblePair(sourceSuperType, targetSuperType, formattingAnnotation);
                // it gets in from here because there is a small amount of time when typeConverters are empty
                typeConverter = typeConverters.get(pair);
                if (typeConverter != null) {
                    converterCache.put(pair, typeConverter);
                    return typeConverter;
                }
            }
        }

Here is what happening on the application context start in DefaultBeanContext#start :

if (initializing.compareAndSet(false, true)) {
     // Reset possibly modified shared context
     ((DefaultConversionService) ConversionService.SHARED).reset(); // <- HERE shared caches a reseted on each start

Looking at the reset method we can see that it is not synchronized despite it changes the state of two global caches:

    public void reset() {
        typeConverters.clear(); // thread 1 exec this
        converterCache.clear(); // thread 1 sleep here
        registerDefaultConverters();
    }
    
    // thread 2 gets null typeConverters.get('micronaut.server.cors.enabled') 
    // thread 2 puts 'micronaut.server.cors.enabled' = UNCONVERTIBLE into converterCache
    // any other thread can read the UNCONVERTIVLE value from converterCache

Proposed fix

Use global shared lock for the reset() method in order to fix the race condition.

@PakhomovAlexander
Copy link
Author

I know that the 4.x version does not have this bug because it does not use the shared converter. But for some users, it is not possible to update up to 4.x version due to its java >= 17 restrictions.

I would like to contribute a fix for the issue in 3.10.x branch. @dstepanov I saw your changes in the codebase, WDYT?

PakhomovAlexander added a commit to unisonteam/ignite-3 that referenced this issue Nov 9, 2023
It happens bacause of the race condition inside
micronaut.

micronaut-projects/micronaut-core#10091
PakhomovAlexander added a commit to apache/ignite-3 that referenced this issue Nov 9, 2023
It happens because of the race conditions inside the micronaut.

micronaut-projects/micronaut-core#10091
Pochatkin pushed a commit to unisonteam/ignite-3 that referenced this issue Nov 9, 2023
@wetted
Copy link
Contributor

wetted commented Nov 20, 2023

@PakhomovAlexander Do you have a solution in mind for this? I'm not confident locking on the reset() method is the correct solution, and we also want to be careful not to introduce additional performance concerns. I think that was part of the reason it was removed for version 4.

ping @dstepanov do you have some thoughts about this?

I have created a reproducible example project with the code provided in the description at https://github.com/wetted/core-10091-race-condition.

@PakhomovAlexander
Copy link
Author

@wetted thanks for your response.

I found a benchmark ConversionServiceBenchmark. I'll try several fixes and come back with the fastest one.

PakhomovAlexander added a commit to PakhomovAlexander/micronaut-core that referenced this issue Nov 21, 2023
@wetted wetted added the status: validated An issue that has been validated as being a bug label Nov 21, 2023
sdelamo pushed a commit that referenced this issue Jan 9, 2024
…on (#10143)

* Optimize cache usage in DefaultConversionService and fix race condition (#10091)

* Fix HttpGetSpec
@wetted wetted closed this as completed Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: validated An issue that has been validated as being a bug
Projects
Status: Done
3 participants