MethodInvokingFactoryBean not well-suited for non-factory methods [SPR-11196]

[spring-projects-issues]

Benoit Lacelle opened SPR-11196 and commented

If a bean holds an @Autowire field, the whole application context seems to be initialized early to check for the type of all beans.

I would expect this eager initialisation phase not to actually instantiate the beans, in order to rely on the default bean ordering.

This seems not to be the case when considering some FactoryBean, at least MethodInvokingFactoryBean.

In my case, the MethodInvokingFactoryBean has been loaded right before the @Autowire field initialisation (when it is suppose to happen much later).

it ssems to be done so as, since this been is not configured yet: MethodInvokingFactoryBean

/**
     * Return the type of object that this FactoryBean creates,
     * or <code>null</code> if not known in advance.
     */
    public Class<?> getObjectType() {
        if (!isPrepared()) {
            // Not fully initialized yet -> return null to indicate "not known yet".
            return null;
        }
        return getPreparedMethod().getReturnType();
    }

Then, it instanciates the underlying singleton bean (i.e. it executes the method):

DefaultListableBeanFactory(DefaultSingletonBeanRegistry).getSingleton(String, ObjectFactory) line: 225
DefaultListableBeanFactory(AbstractBeanFactory).doGetBean(String, Class<T>, Object[], boolean) line: 291
DefaultListableBeanFactory(AbstractBeanFactory).getTypeForFactoryBean(String, RootBeanDefinition) line: 710

I would expect AbstractBeanFactory.doGetBean to check for typeCheckOnly when calling

sharedInstance = getSingleton(beanName, new ObjectFactory<Object>() {
                    public Object getObject() throws BeansException {
                        try {
                            return createBean(beanName, mbd, args);
                        }
                        catch (BeansException ex) {
                            // Explicitly remove instance from singleton cache: It might have been put there
                            // eagerly by the creation process, to allow for circular reference resolution.
                            // Also remove any beans that received a temporary reference to the bean.
                            destroySingleton(beanName);
                            throw ex;
                        }
                    }
                });

like in:

if (!typeCheckOnly) {
    markBeanAsCreated(beanName);
}

and in this case, it would not execute the method.

In my case, it lead to a hard to spot issue: as the method execution failed, the autowiring logic simply rejected this bean as a candidate. Later, the methodInvokingFactoryBean is executed again, but it failed for an obscure reason as the first (failed) call has changed some internal state.

Here is a stack leasing to the first method call:

MyProjectClass(MyProjectClass).myProjectMethod(String) line: myProjectLine
                NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method]    
                NativeMethodAccessorImpl.invoke(Object, Object[]) line: 39  
                DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25         
                Method.invoke(Object, Object...) line: 597        
                MethodInvokingFactoryBean(MethodInvoker).invoke() line: 273           
                MethodInvokingFactoryBean.doInvoke() line: 162         
                MethodInvokingFactoryBean.afterPropertiesSet() line: 152      
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).invokeInitMethods(String, Object, RootBeanDefinition) line: 1514
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).initializeBean(String, Object, RootBeanDefinition) line: 1452
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).doCreateBean(String, RootBeanDefinition, Object[]) line: 519          
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).createBean(String, RootBeanDefinition, Object[]) line: 456          
                AbstractBeanFactory$1.getObject() line: 294     
                DefaultListableBeanFactory(DefaultSingletonBeanRegistry).getSingleton(String, ObjectFactory) line: 225         
                DefaultListableBeanFactory(AbstractBeanFactory).doGetBean(String, Class<T>, Object[], boolean) line: 291    
                DefaultListableBeanFactory(AbstractBeanFactory).getTypeForFactoryBean(String, RootBeanDefinition) line: 1356               
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).getTypeForFactoryBean(String, RootBeanDefinition) line: 710  
                DefaultListableBeanFactory(AbstractBeanFactory).isTypeMatch(String, Class<?>) line: 519        
                DefaultListableBeanFactory.doGetBeanNamesForType(Class<?>, boolean, boolean) line: 339 
                DefaultListableBeanFactory.getBeanNamesForType(Class<?>, boolean, boolean) line: 316       
                BeanFactoryUtils.beanNamesForTypeIncludingAncestors(ListableBeanFactory, Class, boolean, boolean) line: 187               
                DefaultListableBeanFactory.findAutowireCandidates(String, Class<?>, DependencyDescriptor) line: 857           
                DefaultListableBeanFactory.doResolveDependency(DependencyDescriptor, Class<?>, String, Set<String>, TypeConverter) line: 814            
                DefaultListableBeanFactory.resolveDependency(DependencyDescriptor, String, Set<String>, TypeConverter) line: 731        
                AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(Object, String, PropertyValues) line: 485        
                InjectionMetadata.inject(Object, String, PropertyValues) line: 92          
                AutowiredAnnotationBeanPostProcessor.postProcessPropertyValues(PropertyValues, PropertyDescriptor[], Object, String) line: 284               
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).populateBean(String, AbstractBeanDefinition, BeanWrapper) line: 1106          
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).doCreateBean(String, RootBeanDefinition, Object[]) line: 517          
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).createBean(String, RootBeanDefinition, Object[]) line: 456          
                AbstractBeanFactory$1.getObject() line: 294     
                DefaultListableBeanFactory(DefaultSingletonBeanRegistry).getSingleton(String, ObjectFactory) line: 225         
                DefaultListableBeanFactory(AbstractBeanFactory).doGetBean(String, Class<T>, Object[], boolean) line: 291    
                DefaultListableBeanFactory(AbstractBeanFactory).getBean(String) line: 193     
                BeanDefinitionValueResolver.resolveReference(Object, RuntimeBeanReference) line: 323     
                BeanDefinitionValueResolver.resolveValueIfNecessary(Object, Object) line: 107          
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).applyPropertyValues(String, BeanDefinition, BeanWrapper, PropertyValues) line: 1360          
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).populateBean(String, AbstractBeanDefinition, BeanWrapper) line: 1118          
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).doCreateBean(String, RootBeanDefinition, Object[]) line: 517          
                DefaultListableBeanFactory(AbstractAutowireCapableBeanFactory).createBean(String, RootBeanDefinition, Object[]) line: 456          
                AbstractBeanFactory$1.getObject() line: 294     
                DefaultListableBeanFactory(DefaultSingletonBeanRegistry).getSingleton(String, ObjectFactory) line: 225         
                DefaultListableBeanFactory(AbstractBeanFactory).doGetBean(String, Class<T>, Object[], boolean) line: 291    
                DefaultListableBeanFactory(AbstractBeanFactory).getBean(String) line: 193     
                DefaultListableBeanFactory(AbstractBeanFactory).doGetBean(String, Class<T>, Object[], boolean) line: 284    
                DefaultListableBeanFactory(AbstractBeanFactory).getBean(String) line: 193     
                DefaultListableBeanFactory(AbstractBeanFactory).doGetBean(String, Class<T>, Object[], boolean) line: 284    
                DefaultListableBeanFactory(AbstractBeanFactory).getBean(String) line: 193     
                DefaultListableBeanFactory.preInstantiateSingletons() line: 587            
                XmlWebApplicationContext(AbstractApplicationContext).finishBeanFactoryInitialization(ConfigurableListableBeanFactory) line: 925              
                XmlWebApplicationContext(AbstractApplicationContext).refresh() line: 472   
                ContextLoaderListener(ContextLoader).configureAndRefreshWebApplicationContext(ConfigurableWebApplicationContext, ServletContext) line: 383            
                ContextLoaderListener(ContextLoader).initWebApplicationContext(ServletContext) line: 283
                ContextLoaderListener.contextInitialized(ServletContextEvent) line: 111          
                WebAppContext(ContextHandler).callContextInitialized(ServletContextListener, ServletContextEvent) line: 771               
                WebAppContext(ServletContextHandler).callContextInitialized(ServletContextListener, ServletContextEvent) line: 424              
                WebAppContext(ContextHandler).startContext() line: 763        
                WebAppContext(ServletContextHandler).startContext() line: 249         
                WebAppContext.startContext() line: 1250          
                WebAppContext(ContextHandler).doStart() line: 706   
                WebAppContext.doStart() line: 492       
                WebAppContext(AbstractLifeCycle).start() line: 64        
                Server(HandlerWrapper).doStart() line: 95        
                Server.doStart() line: 277            
                Server(AbstractLifeCycle).start() line: 64             
                MreJettyServer.start(int) line: 85            
                MreJettyServer.main(String[]) line: 59 

---

Affects: 3.1.3

Referenced from: commits https://github.com/spring-projects/spring-framework/commit/cf290ab42a22e412eb55db34c5b43aadcb356d3f

1 votes, 4 watchers

[spring-projects-issues]

Mihai Postelnicu commented

I think this also happens in version 3.2.5. I have a MethodInvokingFactoryBean with lazy-init=true and no references, that gets eagerly initialized with instantiation. I too have other beans with @Autowired.

<bean id="startDerby" class="org.springframework.beans.factory.config.MethodInvokingFactoryBean" lazy-init="true" autowire-candidate="false">
    <property name="targetObject" ref="derbyServer" />
    <property name="targetMethod" value="start" />
    <property name="arguments" ref="printWriter" />
</bean>

[spring-projects-issues]

Juergen Hoeller commented

I suppose your reference to the failure because of some internal state was referring to the target method that you're calling, not to MethodInvokingFactoryBean itself? As far as I can tell, the key problem here is that MethodInvokingFactoryBean isn't able to determine the target object type before it actually called the target method...

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Note that, generally, a direct reference via the bean factory method mechanism won't suffer from this problem since it allows for early introspection of the target method, a la:

<bean id="startDerby" factory-bean="derbyServer" factory-method="start" lazy-init="true" autowire-candidate="false">
    <constructor-arg ref="printWriter"/>
</bean>

The only problem in our concrete case here is that Derby's "start" method is declared as void, which makes it getting rejected by Spring's factory method resolution algorithm :-(

Of course, what you can always do is to create a little DerbyStarter class of your own, accepting the Derby NetworkServerControl and a PrintWriter as constructor arguments, and calling that start method programmatically there. Since this is a regular Java class, there won't be any type determination problems and no side effects with by-type autowiring...

Juergen

[spring-projects-issues]

Juergen Hoeller commented

As a more immediate alternative to MethodInvokingFactoryBean (which more or less expects a result from the call to be exposed to the container), we could offer a MethodInvokingBean that just does the method call, with no further expectations. That would be a regular Java class as well, so no type determination effects to consider... And from the perspective of your bean definition code, all you'd have to do is to change the "MethodInvokingFactoryBean" class name to "MethodInvokingBean"... Would that help?

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Since it also serves as a straightforward base class for MethodInvokingFactoryBean, I went ahead and extracted a MethodInvokingBean along those lines. To be available in the next 4.0.3 snapshot. Can also easily be copied and used with older Spring versions.

Juergen

[spring-projects-issues]

Mihai Postelnicu commented

Thanks a lot for explaining what was going on and for the fix!

I think i will upgrade to 4.x anyway, so will give this one a try soon.

Mihai