1

The problem:

The problem appears in a k8s pod running spring-boot, java 8 (more details below)

When using ObjectProvider<> and calling *provider.getObject(.....)*, on a prototype bean defined in Spring Configuration, every now and then (never find a way to make it happen regularly) setter injection methods are not called.

Update oct, 2: See Spring Issue #25840

Most of the time this works perfectly well, but sometimes, it constructs a new object but misses to call the @Autowired setter method (checked with log information).

We also discover that 90% of the time it happens after application startup, but not always.

UPDATE sept, 21: Deleting (Restarting) de pod solves the problem.

UPDATE sept, 22: Once it happens it will happen all the time, I mean is not that it fails once, and the next time it works ok, it will always fail to call setters.

UPDATE sept, 23: New evidence related to concurrency problem in the real application, didn't appear until now, a single thread alone seemed to generate the problem. (take a look at classes below to understand better this description)

ToipParentClass (this is a Strategy Implementation) has setter @Autowired for

  • VlocityService
  • OrderManagemenService

InternetParentClass (this is a Strategy Implementation) has setter @Autowired for

  • VlocityService
  • OrderManagemenService

Log (commented)

[-nio-80-exec-10] GuidTaskController   : Build Strategy using XOM_TOIP_OFFER .                    
[p-nio-80-exec-2] GuidTaskController   : Build Strategy using XOM_INTERNET_OFFER .                    
[-nio-80-exec-10] ToipParentClass      : @Constructing ToipParentClass                                             
[p-nio-80-exec-2] InternetParentClass  : @Constructing InternetParentClass                                         
[-nio-80-exec-10] ToipParentClass      : @Autowiring VlocityServices@7951cd46                                      
[p-nio-80-exec-2] InternetParentClass  : @Autowiring VlocityServices@7951cd46                                      
[-nio-80-exec-10] ToipParentClass      : @Autowiring OrderManagementService@3385326a     
-------------------------------------------------------------
ERROR !!! Missing @Autowiring log  
[p-nio-80-exec-2] InternetParentClass  : @Autowiring OrderManagementService@7951cd46                      
-------------------------------------------------------------
[p-nio-80-exec-2] Controller           : Unexpected Error
2020-09-22T18:56:45.544525916Z
-------------------------------------------------------------
ERROR: NullPointer when using not set OrderManagementService
-------------------------------------------------------------
2020-09-22T18:56:45.544530395Z java.lang.NullPointerException: null
2020-09-22T18:56:45.544534074Z  at InternetParentClass.generateIds(InternetParentClass.java:50) ~[classes!/:BUG001_PrototypeBeanAutowired-8]                                       
2020-09-22T18:56:45.544538568Z  at GuidTaskController.setGUID(GuidTaskController.java:180) ~[classes!/:BUG001_PrototypeBeanAutowired-8]

I made a simple test in https://github.com/toniocus/strategy-calculator run it standalone, different jdk8 versions, also in the same docker image used in the pod (all things in the project), and failed to make it FAIL.

Any ideas on where to look for a problem, suggestions on what to try, or even a solution :-), will be greatly welcome, thanks in advance

Below the product versions, classes.

Detail about versions:

k8s:

v1.14.9-eks-658790

spring-boot:

2.1.4.RELEASE

JDK:

  openjdk version "1.8.0_212"
  OpenJDK Runtime Environment (IcedTea 3.12.0) (Alpine 8.212.04-r0)
  OpenJDK 64-Bit Server VM (build 25.212-b04, mixed mode)

The Classes

A snippet on the classes involved, no real code, do not worry about syntax errors. (real code can be found in https://github.com/toniocus/strategy-calculator)

// ============================================================
public interface Strategy {
   void work(...);
}

// ============================================================
@Service
public class Service {
    public void doSomething() {
       ......
       ......
    }
}

// ============================================================
public class MobileStrategy implements Strategy {
   private Service service;

   @Autowired
   public void setService(Service s) {
       this.service = s;   // setter not called every now and then
   }



   public void work(...) {
       this.service.doSomething();  // throws NullPointerException every now an then
   }
}

// ============================================================
public enum StrategyEnum {

    MOBILE("mobileKey", MobileStrategy.class),
    TV("tvKey", TvStrategy.class),
    .......

    public Class<Strategy> getImplementationClass() {
       ......
    }

    public StrategyEnum findByKey(String key) {
       .....
    }
}

// ============================================================
@Configuration
public class Configuration {

    @Bean
    @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE)
    public Strategy getStrategy(final StrategyEnum selectorEnum) {

        try {

            Constructor<? extends Strategy> constructor =
                    selectorEnum.getImplementationClass().getConstructor();

            return constructor.newInstance();
        }
        catch (Exception ex) {

            throw new Exception("Not able to instantiate interface implementation"
                    + " for enum: " + selectorEnum
                    , ex);
        }

    }
}


// ============================================================
@RestController
public class MathOperRestController {

    @Autowired
    ObjectProvider<Strategy> provider;

    @GetMapping("/{operation}/{x}/{y}")
    public BigDecimal add(
            @PathVariable("operation") final String operation
            , @PathVariable("x") final BigDecimal x
            , @PathVariable("y") final BigDecimal y
            ) {

        Strategy strategy = this.provider.getObject(StrategyEnum.findByKey(operation));
        strategy.doWork(x, y);

        return BigDecimal.ZERO;
    }

UPDATE sept,21 added Service class to samples

tonio
  • 484
  • 5
  • 15
  • 1
    The problem is the fact it is a prototype bean and how configuration classes work. When looking into the framework you will eventually end up in the `AutowiredAnnotationBeanPostProcessor` which does some caching for autowired metadata to speed up things. However, as for each processor, you have the metadata gets refreshed. Now when using a single thread (or synchronized) this isn't an issue as it can complete. However, when using multiple threads the injection metadata is suddenly cleared in one thread and filled in the other. Check the `findAutowiringMetadata` method in the aforementioned cla – M. Deinum Sep 30 '20 at 06:40
  • Not sure if this is stackoverflow style, but a big thanks @M.Deinum for digging in spring for all of us :-) – tonio Oct 01 '20 at 14:43

2 Answers2

1

The problem I found

Update oct, 1: Please read @Denium comment in the question !!!! (thanks for it).

After today's (Sept, 23) update, seems clearly the problem is a concurrency problem, I was able to reproduce it easily (See SimpleSpringAppTest.java) in a Simple Spring app, no spring-boot.

Made all Strategy implementations have the same set of @Autowired setters and the error is still there.

Seems there is some caching done, and @Autowired setters are taken from the cache, and not from the newly constructed object, although I try to dig into spring sources difficult to understand in so short time.

The problem was solved, avoiding concurrency (changes below), so now my question:

Is this the expected behaviour or is it a bug ?

I was not able to find any documentation regarding this problem or describing this behaviour anywhere, so still a question for me.

I tested in spring-boot versions 1.5.22, 2.1.4 (what we are currently using), and 2.3.4 and in all cases the same problem appears, just in a simple spring application, no need of RestController or so.

Workaround 1

Add an intermediate Factory to ensure Strategy beans are created 'synchronously'.

Update oct, 1: After @Deinum comments, from what I understood, Spring will be scanning classes every time (or almost every time) for annotations, so I guess Workaround 2 is probably a better solution.

This solution is more suitable for my current environment.

New Class StrategyFactory

Note the getStrategy(...) method is synchronized, I guess this solution will have some performance impact, but still not able to measure it.

@Component
public class StrategyFactory {

    @Autowired
    ObjectProvider<Strategy> provider;

    public synchronized Strategy getStrategy(final MathOperEnum operation) {
        return this.provider.getObject(operation);
    }

}

Changes in RestController

Now using the StrategyFactory instead of the ObjectProvider

@RestController
public class MathOperRestController {

    @Autowired
    StrategyFactory factory;

    @GetMapping("/{operation}/{x}/{y}")
    public BigDecimal add(
            @PathVariable("operation") final String operation
            , @PathVariable("x") final BigDecimal x
            , @PathVariable("y") final BigDecimal y
            ) {

        Strategy strategy = this.factory
             .getStrategy(StrategyEnum.findByKey(operation));

        strategy.doWork(x, y);

        return BigDecimal.ZERO;
    }
}

Workaround 2

  • Make the StrategyFactory ApplicationContextAware
  • Add @Componente/@Scope annotation to each strategy implementation
  • Remove @Configuration class
@Component
public class StrategyFactory implements ApplicationContextAware {

    private ApplicationContext ctx;


    public Strategy getStrategy(final StrategyEnum operation) {
        return this.ctx.getBean(
               operation.getImplementationClass()
               );
    }


    @Override
    public void setApplicationContext(
         final ApplicationContext applicationContext) 
    throws BeansException {

        this.ctx = applicationContext;

    }

}
tonio
  • 484
  • 5
  • 15
-2

This is not the correct way to create an instance of Strategy bean in Configuration#getStrategy method as it will not call setter using autowiring.

Constructor<? extends Strategy> constructor =
                selectorEnum.getImplementationClass().getConstructor();

return constructor.newInstance();

The way @Autowired is used, it seems you want to create a bean of which instance creation is handled by Spring.

You can refer this answer https://stackoverflow.com/a/52355649/2614885 to use AutowireCapableBeanFactory which creates bean in spring container for the bean id you specify which seems to be either 'mobileKey' or 'tvKey' in your case.

try following code in your @Configuration class

@Configuration
public class Configuration {

    @Autowired private AutowireCapableBeanFactory autowireCapableBeanFactory;

    @Bean
    @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE)
    public Strategy getStrategy(final StrategyEnum selectorEnum) {

        try {

            Constructor<? extends Strategy> constructor =
                    selectorEnum.getImplementationClass().getConstructor();

            Strategy strategyBean = constructor.newInstance();
            autowireCapableBeanFactory.autowireBean(strategyBean);
            return strategyBean;
        }
        catch (Exception ex) {

            throw new Exception("Not able to instantiate interface implementation"
                    + " for enum: " + selectorEnum
                    , ex);
        }

    }
}
Vishal
  • 666
  • 1
  • 8
  • 30
  • Well, beside a discussion if it should be used or not, one thing I can say is it works perfectly well 99% of the times, the fact the the constructor.newInstance() is called inside a @@Configuration class, in a method with @@Bean Annotation is somehow equivalent of doing *new WhateverStrategy()*, which is ok in Configuration classes, may be I'm missing something ? – tonio Sep 21 '20 at 19:45
  • It is not point of best practice only, I dont think how it is working with current implementation because Spring is not aware of this instance which you create with `constructor.newInstance` which make it unable to autowire. The only thing I am wondering is if you are using both spring's component scan and your new instance code, thus creating a race condition – Vishal Sep 21 '20 at 19:50
  • The answer is yes, Singleton components (in my example the Service class) *are annotated and "obtained" (???) using component scan*, while (Prototype) Strategy Components are *NOT Annotated and 'obtained' using @@Configuration class*, the Application class is annotated only with @@SpringBootApplication, and everything things to work ok in the real application, wondering if you can give me more details on the race condition you mention I guess the "occasional* error might have to do with it, thanks – tonio Sep 21 '20 at 20:24
  • One thing to mention **when everything works ok** I see in the log: /Constructing Object/Autowiring Service1/Autowiring Service2, while **when it fails** I only see /Constructing Object, so kind of lost why is this happening, race condition must be the reason perhaps – tonio Sep 21 '20 at 20:32
  • 1
    My understanding is that **this answer's initial statement is patently wrong**. Any method marked with `@Bean` in a class that is scanned for whatever reason (like being annotated with `@Configuration`) that returns an object will cause that object to be managed by Spring and itself scanned for annotations, including `@Autowired` annotations. It should make no difference how the object returned by a `@Bean` method is constructed, and it surely does not need to be created via a `BeanFactory`, because in that case, it would already be a Spring Bean, so what would the `@Bean` annotation do? – CryptoFool Sep 21 '20 at 23:06
  • 1
    Straight from the Spring docs `Spring @Bean Annotation is applied on a method to specify that it returns a bean to be managed by Spring context. Spring Bean annotation is usually declared in Configuration classes methods.` - You can look for examples of the use of @Bean on the internet. Such annotated methods almost always do `return new Foo();`. – CryptoFool Sep 21 '20 at 23:06
  • It is not only about @Bean annotation. I don't think setter injection will fire when a new instance is made with `constructor.newInstance()`. Even for constructor injection, if you are using manual bean creation rather than component scan you have to provide the constructor params which itself could be beans. Main point is in component scan, spring gathers it dependencies on its own but if you are creating instance on your own to be considered as bean, you need to provide dependencies by yourself – Vishal Sep 22 '20 at 08:15
  • Vishal, it does, you can try my sample, thanks for your help – tonio Sep 22 '20 at 13:27