John Wu opened SPR-6353 and commented

The Symptom

  • Each test passes when running individually;
  • A few @Transactional tests may fail when running in a test suite. The failure may occur on any @Transactional test class, depending on the nondeterministic runtime tests sequence. In fact, it's quite difficult to reproduce before knowing the root cause.

Steps to Reproduce

  • Using Spring 2.5.6 or 3.0.0.RC1, JUnit 4;
  • Create a SimpleDbOpService (may or may not annotated with @Transactional), add a method to insert a record into DB based on the input parameters;
  • Create a SimpleDbOpATest:
@ContextConfiguration({"base.xml", "A.xml"})
  • Create a SimpleDbOpBTest:
@ContextConfiguration({"base.xml", "B.xml"})
  • Create a SimpleDbOpCTest:
@ContextConfiguration({"base.xml", "A.xml"})
  • Each test is annotated with @Transactional, calling the SimpleDbOpService and verifying the DB operation results at the end of test method;
  • <tx:annotation-driven mode="aspectj"/> in base.xml, also add "transactionManager" definition in
  • Create a test suite class as the following:
    import org.junit.runner.RunWith;
    import org.junit.runners.Suite;

    @RunWith(Suite.class)
    @Suite.SuiteClasses({
      SimpleDbOpATest.class,
      SimpleDbOpBTest.class,
      SimpleDbOpCTest.class
    })
    public class Debug {

    }
  • The tests in SimpleDbOpCTest will fail.

Root Cause

In the Test Framework, it creates one instance of ApplicationContext for each "Set of context locations" and caches (and switches) the ApplicationContext instances by "context locations". Among all those ApplicationContext instances, the instance of AnnotationTransactionAspect is shared. That is, during the test suite running, there will be only one instance of AnnotationTransactionAspect, no matter how many ApplicationContext instances created.

A reference to BeanFactory (in spring 3.0.0.RC) or transactionManager (in spring 2.5.6) will be injected into AnnotationTransactionAspect (derived from TransactionAspectSupport) while the ApplicationContext being loaded.

In the example above, test A and C have the exactly same context locations, and they will share the same application context instance when running in a test suite. So, when running tests A, B, and C in a suite and in that order, the application context instance switches from A to B to A. However, the transactionManager instance (retrieved from an instance of AnnotationTransactionAspect) will be A, B, and B (should be A though), because there is only one instance of AnnotationTransactionAspect per class loader. In turn, that causes the operations in the C.testXxx() be split in to two transactionManager instances, one from AnnotationTransactionAspect and the other from ApplicationContext. Therefore, the DB result verification fails.

Proposed Solution

To create one Aspect instance per ApplicationContext, not per class loader. Not sure if that's achievable though.

Further Resources

Related Issues

  • 11897

  • 17123

  • 10789

  • 12619

  • 24794

  • 30229

Affects: 2.5.6, 3.0.5

Reference URL: http://forum.springsource.org/showthread.php?t=79949

5 votes, 7 watchers

Comment From: sbrannen

Added "waiting for triage" label in order to assess whether this is still an issue with recent versions of the framework.

Comment From: cdalexndr

Encountered a problem that impacts @Cacheable and @CacheEvict, and can cause inconsistent cache: Consider a service with two methods, one annotated @Cacheable, other @CacheEvict. 1. First app context is created, AnnotationCacheAspect.cacheResolver is first set to null from CacheAspectSupport.configure method where all params are null 2. AnnotationCacheAspect.cacheResolver is set to correct bean in method CacheAspectSupport.afterSingletonsInstantiated 3. Test method uses first method (@Cacheable) and puts a key in CacheAspectSupport.metadataCache with the cacheResolver from (1) in method CacheAspectSupport.getCacheOperationMetadata (lazy initialized as methods annotated @Cacheable/@CacheEvict are used) 4. New app context is created. Again AnnotationCacheAspect.cacheResolver is set to null. AnnotationCacheAspectis the same instance as the one from previous app context (!problem!) 5. AnnotationCacheAspect.cacheResolver is initialized to another bean from second app context 6. Test method uses second method (@CacheEvict) and puts a key in the same CacheAspectSupport.metadataCache with a cacheResolver initialized above, that is different from the key corresponding to the first service method with @Cacheable 7. Test method calls first service method, and it expects to re-compute the values and put in cache because it was evicted previously... instead it incorrectly gets the cached value from the first cacheResolver cache

spring-aspects-5.1.8.RELEASE

Comment From: cdalexndr

This issue is 10 years old, why is there no initiative to fix this as it creates multiple problems?

Comment From: cdalexndr

Note that this is still an issue with 5.2.8.RELEASE. Should remove waiting-for-triage tag. For sample, see one of my linked issues with test repository and update spring version.

Workaround originally from Łukasz Świątek (https://github.com/spring-projects/spring-framework/issues/20775) and enhanced by me:

@Order(Ordered.HIGHEST_PRECEDENCE)
@TestConfiguration
public class AspectjWorkaroundTestListener extends AbstractTestExecutionListener {
    private static final Logger log = LoggerFactory.getLogger( AspectjWorkaroundTestListener.class );

    @Order(Ordered.HIGHEST_PRECEDENCE)
    @EventListener(ContextRefreshedEvent.class)
    public void onEvent( ApplicationContextEvent ev ) {
        //fix context before any events are processed
        fixAppContext( ev.getApplicationContext() );
    }

    @Override public void beforeTestMethod( TestContext testContext ) {
        //methods may run intertwined between contexts
        fixAppContext( testContext.getApplicationContext() );
    }

    public static void fixAppContext( ApplicationContext appCtx ) {
        try {
            if (appCtx instanceof ConfigurableApplicationContext) {
                ConfigurableApplicationContext configurableAppCtx = (ConfigurableApplicationContext) appCtx;
                ConfigurableListableBeanFactory bf = configurableAppCtx.getBeanFactory();

                //workaround for @Configurable
                AnnotationBeanConfigurerAspect configurableAspect = appCtx.getBean( AnnotationBeanConfigurerAspect.class );
                configurableAspect.destroy();
                configurableAspect.setBeanFactory( bf );

                //workaround for @Transactional
                AnnotationTransactionAspect transactionAspect = bf.getBean( AnnotationTransactionAspect.class );
                transactionAspect.destroy();
                transactionAspect.setBeanFactory( bf );

                //workaround for @Cacheable/@CacheEvict
                AnnotationCacheAspect cacheAspect = bf.getBean( AnnotationCacheAspect.class );
                cacheAspect.destroy();
                cacheAspect.setBeanFactory( bf );

                updateMyAspects( bf );
            } else {
                log.warn( String.format( "Test application context not an instance of ConfigurableApplicationContext, was %s", appCtx.getClass() ) );
            }
        } catch (RuntimeException exc) {
            log.warn( "Cannot fix app context", exc );
        }
    }

    private static void updateMyAspects( BeanFactory beanFactory ) {
        Aspects.aspectOf( MyAspect1.class ).setBeanFactory( beanFactory );
        Aspects.aspectOf( MyAspect2.class ).setMaintenanceService( beanFactory.getBean( MaintenanceService.class ) );
        ...
    }
}

With the following two helper classes, one for transactional tests, other for non transactional test classes.

@TestExecutionListeners(AspectjWorkaroundTestListener.class)
@Import(AspectjWorkaroundTestListener.class)
public class Test extends AbstractTestNGSpringContextTests {
}

@TestExecutionListeners(AspectjWorkaroundTestListener.class)
@Import(AspectjWorkaroundTestListener.class)
public class TransactionalTest extends AbstractTransactionalTestNGSpringContextTests {
}

Usage:

@SpringBootTest
public class MyTestClass extends TransactionalTest {
    ...
}

Comment From: sbrannen

Update: currently investigating options to address issues related to the following aspects.

  • AnnotationBeanConfigurerAspect
  • AnnotationAsyncExecutionAspect
  • AnnotationCacheAspect
  • AnnotationTransactionAspect
  • JtaAnnotationTransactionAspect

Comment From: JohnMTu

Hello,

every few years on new project i hit this issue. I'm actually using similar workaround as posted here.

isn't actually correct solution to simply change that aspect to be "pertarget" ? https://www.eclipse.org/aspectj/doc/released/progguide/semantics-aspects.html#aspect-instantiation

That way, as each spring context has own instance of object, will have also own aspect. Each with correct beanfactory etc. No need to cleanup, reinit etc.

best regards

Comment From: JohnMTu

@sbrannen would there be an issue to have aspect "pertarget" ?

Comment From: ahaczewski

Am I the only one that thinks Spring Framework no longer cares for good AspectJ support? We (me with team) have spent too much time (days...) investigating the reason our tests fail, and after we discovered the reason I found this issue... open since 2009... 😞

Comment From: TheQuiu

same issue, when fix