AnnotationTransactionAspect retains reference to JpaTransactionManager from closed context [SPR-12518]

[spring-projects-issues]

Marek Wiącek opened SPR-12518 and commented

Original Input From Reporter

@DirtiesContext is broken in 4.1.0:

spring-test fails to recreate a context containing a JPA EntityManager if a test class is annotated with @DirtiesContext(classMode = ClassMode.AFTER_EACH_TEST_METHOD). Interestingly, this only happens in AspectJ mode for @Transactional support.

See the attached program to see how to reproduce the problem.

Downgrading the version number in build.gradle to 4.0.8.RELEASE fixes this problem. Apparently this is a regression introduced in 4.1.0.RELEASE.

---

Analysis

The behavior experienced in the example project is a direct result of the caching introduced in #16570.

Specifically, the determineTransactionManager() method in TransactionAspectSupport (which is a superclass of AnnotationTransactionAspect which in turn provides AspectJ-based support for @Transactional in Spring) now retains a reference to the PlatformTransactionManager. Since the aspect is a singleton within the class loader, its internal state does not get automatically reset when the application context is closed via @DirtiesContext semantics.

In other words, the testing framework does in fact close the application context due to @DirtiesContext semantics, but the AnnotationTransactionAspect retains a reference to the JpaTransactionManager from the closed application context, and that transaction manager in turn retains a reference to the closed EntityManagerFactory. This is evident in the resulting stack trace.

org.springframework.transaction.CannotCreateTransactionException: Could not open JPA EntityManager for transaction; nested exception is java.lang.IllegalStateException: EntityManagerFactory is closed
	at org.springframework.orm.jpa.JpaTransactionManager.doBegin(JpaTransactionManager.java:431)
	at org.springframework.transaction.support.AbstractPlatformTransactionManager.getTransaction(AbstractPlatformTransactionManager.java:373)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.createTransactionIfNecessary(TransactionAspectSupport.java:439)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:262)
	at org.springframework.transaction.aspectj.AbstractTransactionAspect.ajc$around$org_springframework_transaction_aspectj_AbstractTransactionAspect$1$2a73e96c(AbstractTransactionAspect.aj:64)
	at EntryDaoImpl.saveEntry(EntryDaoImpl.java:14)
	at EntryDaoTest.testSaveEntry(EntryDaoTest.java:21)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:73)
	at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:82)
	at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:73)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:217)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:83)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:68)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:163)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.runTestClass(JUnitTestClassExecuter.java:86)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.execute(JUnitTestClassExecuter.java:49)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassProcessor.processTestClass(JUnitTestClassProcessor.java:69)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:48)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
	at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.messaging.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:32)
	at org.gradle.messaging.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
	at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:105)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
	at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.messaging.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:360)
	at org.gradle.internal.concurrent.DefaultExecutorFactory$StoppableExecutorImpl$1.run(DefaultExecutorFactory.java:64)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IllegalStateException: EntityManagerFactory is closed
	at org.hibernate.jpa.internal.EntityManagerFactoryImpl.validateNotClosed(EntityManagerFactoryImpl.java:388)
	at org.hibernate.jpa.internal.EntityManagerFactoryImpl.internalCreateEntityManager(EntityManagerFactoryImpl.java:342)
	at org.hibernate.jpa.internal.EntityManagerFactoryImpl.createEntityManager(EntityManagerFactoryImpl.java:313)
	at org.springframework.orm.jpa.JpaTransactionManager.createEntityManagerForTransaction(JpaTransactionManager.java:449)
	at org.springframework.orm.jpa.JpaTransactionManager.doBegin(JpaTransactionManager.java:369)
	... 54 more

---

Deliverables

1. TBD

---

Affects: 4.1 GA

Attachments:

• spring-jpa-tx.tar.bz2 (51.51 kB)

Issue Links:

• Reduce PlatformTransactionManager lookups in TransactionAspectSupport [SPR-11954] #16570 Reduce PlatformTransactionManager lookups in TransactionAspectSupport ("depends on")
• Dependency injection of @Configurable objects should work across test suites [SPR-6121] #10789 Dependency injection of @Configurable objects should work across test suites
• TestContext framework should support one AspectJ instance per ApplicationContext [SPR-6353] #11019 TestContext framework should support one AspectJ instance per ApplicationContext
• AnnotationTransactionAspect retains reference to closed BeanFactory [SPR-7964] #12619 AnnotationTransactionAspect retains reference to closed BeanFactory
• @Transactional qualifier is ignored by TransactionAspectSupport if default transaction manager is set [SPR-12541] #17145 @Transactional qualifier is ignored by TransactionAspectSupport if default transaction manager is set

Referenced from: commits ca91956, fd7153f

0 votes, 6 watchers

[spring-projects-issues]

Sam Brannen commented

@DirtiesContext is not broken per se. In other words, nothing has changed with regard to support for @DirtiesContext in the testing framework. However, I was able to confirm the regression you have discovered.

So, thanks for submitting the example project!

Please see the Analysis in this issue's updated description for further details.

[spring-projects-issues]

Sam Brannen commented

I'm still pondering what the best solution to this issue is; however, in the meantime I have a work-around for you.

In your Gradle build configuration:

project.ext {
    springVersion = "4.1.2.RELEASE"
    // ...
}

// ...
dependencies {
    // ...
    testCompile "org.springframework:spring-aspects:$springVersion"
    // ...
}

In EntryDaoTest, add the following method.

@After
public void cleanUpAspects() {
     AnnotationTransactionAspect.aspectOf().setTransactionManager(null);
}

Let me know if that works for you!

Thanks,

Sam

[spring-projects-issues]

Marek Wiącek commented

Invoking AnnotationTransactionAspect.aspectOf().setTransactionManager(null) fixes the problem, thank you.

[spring-projects-issues]

Sam Brannen commented

Thanks for providing feedback, Marek!

And... I'm glad that work-around fixes the issue for you. :)

[spring-projects-issues]

Stéphane Nicoll commented

We had the same issue with the Cache operation metadata that are .... cached as well. It was only a problem in test really; exactly as in this case. It turns out that we fixed that problem by making the aspect aware that the context has been closed. We can apply the same trick here.

[spring-projects-issues]

Sam Brannen commented

Sounds good!

[spring-projects-issues]

Stéphane Nicoll commented

Applied the same trick as the clearMetadataCache of the cache aspect so that all transaction managers are cleared from the cache when the context is disposed. This avoids any hollow reference to a transaction manager defined from a context that has been closed.

[spring-projects-issues]

Sam Brannen commented

Looks Good, Stéphane. Thanks!

[spring-projects-issues]

Turbanov Andrey commented

@snicoll Could you please update Fix version in JIRA to 4.2.2?
As I can see this issue was fully fixed only in that version.
It is necessary to convince my colleagues to update Spring version)