Spring Disposable bean registration misses changes introduced by post-processing

I'm on Spring Boot 3.1.2. Whenever I have a shutdown() method on a bean, which will be called by Spring's IOC container, it looks like Spring is actually shutting down original bean, not a spy. To demonstrate:

FlagBeanConfig.java

@Configuration
class FlagBeanConfig {
    @Bean
    FlagBean spyFlagBean() {
        return new FlagBean("spy");
    }

    @Bean
    FlagBean autowiredFlagBean() {
        return new FlagBean("autowired");
    }
}

FlagBean.java

class FlagBean {
    private boolean flag;
    private String name;

    FlagBean(String name) {
        this.name = name;
    }

    void changeFlag() {
        this.flag = true;
        System.out.printf("%s starts: The flag was set to: %b%n", name, flag);
    }

    public void shutdown() {
        System.out.printf("%s shuts down: The flag was: %b%n", name, flag);
    }
}

FlagBeanTest.java

@SpringBootTest(classes = FlagBeanConfig.class)
class FlagBeanTest {

    @SpyBean
    @Qualifier("spyFlagBean")
    private FlagBean spyFlagBean;

    @Autowired
    @Qualifier("autowiredFlagBean")
    private FlagBean autowiredFlagBean;

    @Test
    void hello() {
        spyFlagBean.changeFlag();
        autowiredFlagBean.changeFlag();
        System.out.println("Manually shutting down spy");
        spyFlagBean.shutdown();
        System.out.println("Spy was shut down");
    }
}

The test yields the following result:

spy starts: The flag was set to: true
autowired starts: The flag was set to: true
Manually shutting down spy
spy shuts down: The flag was: true
Spy was shut down
autowired shuts down: The flag was: true
spy shuts down: The flag was: false

The last line shows the flag to be false, while in reality it is true. Am I missing something?

Comment From: wilkinsona

Thanks for the sample. Shutdown is being called on the original bean because the spy is created through a bean post-processor but Spring Framework uses the original, unprocessed bean when creating the disposable bean adapter that calls shutdown. This behavior can be reproduced without involving Spring Boot:

package example;

import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import org.springframework.beans.BeansException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;

@SpringJUnitConfig
class FlagBeanTest {

    @Autowired
    @Qualifier("spyFlagBean")
    private FlagBean spyFlagBean;

    @Autowired
    @Qualifier("autowiredFlagBean")
    private FlagBean autowiredFlagBean;

    @Test
    void hello() {
        this.spyFlagBean.changeFlag();
        this.autowiredFlagBean.changeFlag();
        System.out.println("Manually shutting down spy");
        this.spyFlagBean.shutdown();
        System.out.println("Spy was shut down");
    }

    static class FlagBean {

        private boolean flag;

        private String name;

        FlagBean(String name) {
            this.name = name;
        }

        void changeFlag() {
            this.flag = true;
            System.out.printf("%s " + this + " starts: The flag was set to: %b%n", this.name, this.flag);
        }

        public void shutdown() {
            System.out.printf("%s " + this + " shuts down: The flag was: %b%n", this.name, this.flag);
        }

    }

    @Configuration
    static class FlagBeanConfig {

        @Bean
        FlagBean spyFlagBean() {
            return new FlagBean("spy");
        }

        @Bean
        FlagBean autowiredFlagBean() {
            return new FlagBean("autowired");
        }

        @Bean
        static BeanPostProcessor spy() {
            return new BeanPostProcessor() {

                @Override
                public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
                    if (beanName.equals("spyFlagBean")) {
                        return Mockito.spy(bean);
                    }
                    return bean;
                }

            };
        }

    }

}

This feels like a Framework bug to me. In addition to the wrong shutdown method being called in this case, it's also possible that a shutdown method that was introduced by a bean post-processor decorating the original bean would be missed.

We'll transfer this to the Framework team for their consideration.

Comment From: snicoll

Thanks for the report and the reduced sample. I can see that the post-processor has created the spy in time for the DisposableBeanAdapter registration callback, but the original bean instance is registered, rather than the post-processed one.

I've changed this line https://github.com/spring-projects/spring-framework/blob/3d248607dc461eeb69ec9fea9d61e5378d5021d1/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java#L641

To specify the exposedObject rather than the "raw" bean. It did fix the issue for you but several tests fail as they're testing the exact opposite scenario (i.e. the destroy method "should" be called on the raw bean, not the one that's exposed by the post-processor).

In your case, I can totally see why destroy should be called on the exposed bean (as it delegates to the raw instance) and I don't know if there's a way to support both.

@jhoeller can you share why the code behaves this way now?

Comment From: snicoll

I've discussed this with @jhoeller and things are working as designed. The rationale of using the raw bean, and not the bean instance that's ultimately exposed, is that we want to make sure to dispose the bean instance that we've created. It could be such that the proxy exposes a shutdown method that doesn't delegate to the raw instance that we've created, or does not expose a shutdown method (while the raw instance does). It feels to us that using the raw bean provides the best outcome.

Besides, we're not sure what this test above is supposed to do, testing that the shutdown method has been called by the container feels like testing the framework.