Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction manager bean in TransactionInterceptor retained after JUnit test class completes [SPR-14511] #19080

Closed
spring-issuemaster opened this issue Jul 25, 2016 · 9 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Jul 25, 2016

Will Darby opened SPR-14511 and commented

I am migrating from Spring Framework 3.2.15 to 4.3.1. I have a JUnit Test Suite with ~800 tests, and started running into out of heap memory errors after changing to 4.3.1. I compared .hprof dumps to the 3.2.15 version and the difference appears to be that CglibAopProxy$ProxyCallbackFitler are created for each test class with @Transactional annotation (or @Transactional @Test methods). This then retains TransactionInterceptor Advice, which eventually references the HibernateTransactionManager.

The CglibAopProxy$ProxyCallbackFitler is referenced by the CGLIB$CALLBACK_FILTER field of the CGLIB enhanced test class, which is referenced by the AppClassLoader.

These ProxyCallbackFilters were not retained in Spring 3.2.15, thus it appears to be an introduced leak.

Below are 2 HPROF dumps. The first shows the reference to ProxyCallbackFilter from AppClassLoader. The second shows the reference to HibernateTransactionManager from ProxyCallbackFilter.

Class Name                                                                                                                                     | Shallow Heap | Retained Heap
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
org.springframework.aop.framework.CglibAopProxy$ProxyCallbackFilter @ 0xee643418                                                               |           24 |     3,199,752
|- referent org.springframework.cglib.core.WeakCacheKey @ 0xedcec6f8                                                                           |           32 |            32
|- CGLIB$CALLBACK_FILTER class test.com.leeriver.trading.pjm.eDataFeed.ConstraintPriceUpdateTest2$$EnhancerBySpringCGLIB$$ac9e61d9 @ 0xee6427b0|          128 |         3,088
|  |- [11536] java.lang.Object[20480] @ 0xe203fe38                                                                                             |       81,936 |   215,507,136
|  |  '- elementData java.util.Vector @ 0xe0153e20                                                                                             |           32 |   215,507,168
|  |     '- classes sun.misc.Launcher$AppClassLoader @ 0xe00d5700 Native Stack                                                                 |           88 |   216,679,856
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Class Name                                                                                                                            | Shallow Heap | Retained Heap
---------------------------------------------------------------------------------------------------------------------------------------------------------------------
org.springframework.aop.framework.CglibAopProxy$ProxyCallbackFilter @ 0xee643418                                                      |           24 |     3,199,752
|- <class> class org.springframework.aop.framework.CglibAopProxy$ProxyCallbackFilter @ 0xe1bbb878                                     |            0 |             0
|- advised org.springframework.aop.framework.AdvisedSupport @ 0xee643430                                                              |           48 |     3,199,728
|  |- <class> class org.springframework.aop.framework.AdvisedSupport @ 0xe0e21850                                                     |           16 |            16
|  |- targetSource org.springframework.aop.target.EmptyTargetSource @ 0xee643460                                                      |           24 |            24
|  |- advisorChainFactory org.springframework.aop.framework.DefaultAdvisorChainFactory @ 0xee643478                                   |           16 |            16
|  |- methodCache java.util.concurrent.ConcurrentHashMap @ 0xee643488                                                                 |           64 |         1,568
|  |- interfaces java.util.ArrayList @ 0xee643e88                                                                                     |           24 |            24
|  |- advisors java.util.LinkedList @ 0xee643ea0                                                                                      |           32 |            56
|  |- advisorArray org.springframework.aop.Advisor[1] @ 0xee643ed8                                                                    |           24 |            24
|  |  |- <class> class org.springframework.aop.Advisor[] @ 0xe0e01070                                                                 |            0 |             0
|  |  |- [0] org.springframework.transaction.interceptor.BeanFactoryTransactionAttributeSourceAdvisor @ 0xee5cc2f0                    |           40 |            64
|  |  |  |- <class> class org.springframework.transaction.interceptor.BeanFactoryTransactionAttributeSourceAdvisor @ 0xe0de84c8       |            0 |             0
|  |  |  |- order java.lang.Integer @ 0xe00cc4d0  10                                                                                  |           16 |            16
|  |  |  |- beanFactory org.springframework.beans.factory.support.DefaultListableBeanFactory @ 0xee5907a8                             |          208 |       202,624
|  |  |  |- adviceMonitor java.util.concurrent.ConcurrentHashMap @ 0xee590ad8                                                         |           64 |         2,128
|  |  |  |- adviceBeanName java.lang.String @ 0xee592e08  org.springframework.transaction.interceptor.TransactionInterceptor#0        |           24 |           176
|  |  |  |- transactionAttributeSource org.springframework.transaction.annotation.AnnotationTransactionAttributeSource @ 0xee5b4898   |           32 |       206,552
|  |  |  |- pointcut org.springframework.transaction.interceptor.BeanFactoryTransactionAttributeSourceAdvisor$1 @ 0xee5cc318          |           24 |            24
|  |  |  |- advice org.springframework.transaction.interceptor.TransactionInterceptor @ 0xee6438d8                                    |           40 |     2,784,376
|  |  |  |  |- <class> class org.springframework.transaction.interceptor.TransactionInterceptor @ 0xe0de9ed0                          |            0 |             0
|  |  |  |  |- logger org.apache.commons.logging.impl.SLF4JLocationAwareLog @ 0xe137ddf0                                              |           24 |            24
|  |  |  |  |- transactionManagerBeanName java.lang.String @ 0xee2f4da8  transactionManager                                           |           24 |            80
|  |  |  |  |- beanFactory org.springframework.beans.factory.support.DefaultListableBeanFactory @ 0xee5907a8                          |          208 |       202,624
|  |  |  |  |- transactionAttributeSource org.springframework.transaction.annotation.AnnotationTransactionAttributeSource @ 0xee5b4898|           32 |       206,552
|  |  |  |  |- DEFAULT_TRANSACTION_MANAGER_KEY java.lang.Object @ 0xee643900                                                          |           16 |            16
|  |  |  |  |- transactionManagerCache java.util.concurrent.ConcurrentHashMap @ 0xee643910                                            |           64 |     2,784,320
|  |  |  |  |  |- <class> class java.util.concurrent.ConcurrentHashMap @ 0xe0002928 System Class                                      |          136 |           616
|  |  |  |  |  |- table java.util.concurrent.ConcurrentHashMap$Node[16] @ 0xee643950                                                  |           80 |     2,784,256
|  |  |  |  |  |  |- <class> class java.util.concurrent.ConcurrentHashMap$Node[] @ 0xe00c82d8                                         |            0 |             0
|  |  |  |  |  |  |- [10] java.util.concurrent.ConcurrentHashMap$Node @ 0xee6439a0                                                    |           32 |     2,784,176
|  |  |  |  |  |  |  |- <class> class java.util.concurrent.ConcurrentHashMap$Node @ 0xe0253a50 System Class                           |            0 |             0
|  |  |  |  |  |  |  |- key java.lang.String @ 0xee2f4da8  transactionManager                                                         |           24 |            80
|  |  |  |  |  |  |  |- val org.springframework.orm.hibernate5.HibernateTransactionManager @ 0xee6439c0                               |           56 |     2,784,144
---------------------------------------------------------------------------------------------------------------------------------------------------------------------


Affects: 4.3.1

Issue Links:

  • #17145 @Transactional qualifier is ignored by TransactionAspectSupport if default transaction manager is set
  • #16570 Reduce PlatformTransactionManager lookups in TransactionAspectSupport
  • #19177 Transaction manager cache fails to repopulate when multiple transaction managers defined
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 25, 2016

Juergen Hoeller commented

Do you have a corresponding profiler run for 3.2.x as well? Does the transaction interceptor and its transaction manager reference (which is a straight field in that branch) not appear there at all?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2016

Will Darby commented

Thanks for following up. Yes, the transactionManager field of the TransactionInterceptor is null in 3.2.

Here is the 3.2 HPROF trace from the AppClassLoader to the TransactionInterceptor:

Class Name                                                                                                                                      | Shallow Heap | Retained Heap | Percentage
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
sun.misc.Launcher$AppClassLoader @ 0x7824039e0                                                                                                  |           80 |     3,695,736 |     48.40%
|- java.util.Vector @ 0x7824a5518                                                                                                               |           32 |     2,414,872 |     31.62%
|  '- java.lang.Object[5120] @ 0x7dedf00a0                                                                                                      |       20,496 |     2,414,840 |     31.62%
|     |- class org.hsqldb.store.ValuePool @ 0x781d40680                                                                                         |           96 |       789,840 |     10.34%
|     |- class org.springframework.beans.CachedIntrospectionResults @ 0x781e2f5b8                                                               |           24 |       307,552 |      4.03%
|     |- class org.springframework.cglib.proxy.Enhancer @ 0x7dee49dc8                                                                           |          136 |       242,136 |      3.17%
|     |  |- org.springframework.cglib.core.AbstractClassGenerator$Source @ 0x7dee37a60                                                          |           24 |       238,960 |      3.13%
|     |  |  |- java.util.WeakHashMap @ 0x7dee37a78                                                                                              |           56 |       238,816 |      3.13%
|     |  |  |  |- java.util.WeakHashMap$Entry[16] @ 0x7dee37ab0                                                                                 |           80 |       238,712 |      3.13%
|     |  |  |  |  '- java.util.WeakHashMap$Entry @ 0x7dee37b00                                                                                  |           40 |       238,632 |      3.12%
|     |  |  |  |     '- java.util.HashMap @ 0x7dee37b58                                                                                         |           48 |       238,592 |      3.12%
|     |  |  |  |        '- java.util.HashMap$Entry[16] @ 0x7dee37b88                                                                            |           80 |       238,544 |      3.12%
|     |  |  |  |           |- java.util.HashMap$Entry @ 0x7dee37bd8                                                                             |           32 |       238,032 |      3.12%
|     |  |  |  |           |  |- org.springframework.cglib.proxy.Enhancer$EnhancerKey$$KeyFactoryByCGLIB$$4ce19e8f @ 0x7dee37bf8                |           40 |       237,968 |      3.12%
|     |  |  |  |           |  |  |- org.springframework.aop.framework.CglibAopProxy$ProxyCallbackFilter @ 0x7dee37c38                           |           24 |       236,608 |      3.10%
|     |  |  |  |           |  |  |  '- org.springframework.aop.framework.AdvisedSupport @ 0x7dee4fc70                                           |           48 |       236,584 |      3.10%
|     |  |  |  |           |  |  |     |- org.springframework.beans.factory.support.DefaultListableBeanFactory @ 0x78249edf8                    |          200 |       128,872 |      1.69%
|     |  |  |  |           |  |  |     |- org.springframework.transaction.annotation.AnnotationTransactionAttributeSource @ 0x7debc6900         |           32 |       104,584 |      1.37%
|     |  |  |  |           |  |  |     |- java.util.concurrent.ConcurrentHashMap @ 0x7dee4fcb8                                                  |           48 |         2,408 |      0.03%
|     |  |  |  |           |  |  |     |- java.lang.String @ 0x7824935d8  org.springframework.transaction.interceptor.TransactionInterceptor#0  |           24 |           176 |      0.00%
|     |  |  |  |           |  |  |     |- java.lang.String @ 0x7820f9e48  transactionManager                                                    |           24 |            80 |      0.00%
|     |  |  |  |           |  |  |     |- java.lang.reflect.Method @ 0x7debfcd00  protected native java.lang.Object java.lang.Object.clone()    |           80 |            80 |      0.00%
|     |  |  |  |           |  |  |     |- org.springframework.transaction.interceptor.BeanFactoryTransactionAttributeSourceAdvisor @ 0x7dec01b00|           40 |            80 |      0.00%
|     |  |  |  |           |  |  |     |- java.lang.reflect.Method @ 0x7dec25f40  public java.lang.String java.lang.Object.toString()           |           80 |            80 |      0.00%
|     |  |  |  |           |  |  |     |- java.util.LinkedList @ 0x7dec01ac8                                                                    |           32 |            56 |      0.00%
|     |  |  |  |           |  |  |     |- org.springframework.transaction.interceptor.TransactionInterceptor @ 0x7debc6888                      |           32 |            32 |      0.00%
|     |  |  |  |           |  |  |     |- java.util.ArrayList @ 0x7dec01ab0                                                                     |           24 |            24 |      0.00%
|     |  |  |  |           |  |  |     |- org.springframework.aop.Advisor[1] @ 0x7dee4ac40                                                      |           24 |            24 |      0.00%
|     |  |  |  |           |  |  |     |- org.springframework.aop.target.EmptyTargetSource @ 0x7dee4fca0                                        |           24 |            24 |      0.00%
|     |  |  |  |           |  |  |     |- org.springframework.aop.framework.DefaultAdvisorChainFactory @ 0x7dec01848                            |           16 |            16 |      0.00%
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

and the fields from the TransactionInterceptor:

Type|Name                      |Value
-----------------------------------------------------------------------------------------------------------------------------
ref |beanFactory               |org.springframework.beans.factory.support.DefaultListableBeanFactory @ 0x78249edf8
ref |transactionAttributeSource|org.springframework.transaction.annotation.AnnotationTransactionAttributeSource @ 0x7debc6900
ref |transactionManager        |null
ref |transactionManagerBeanName|transactionManager
ref |logger                    |org.apache.commons.logging.impl.SLF4JLocationAwareLog @ 0x7debc68a8
-----------------------------------------------------------------------------------------------------------------------------

I did add a custom TestContextManager in 3,2 (also used in the 4.3 version), which clears the post-bean processors after the test completes to remove the HibernateTransactionManager reference to garbage collect between tests. It appears in 4.3 the transactionManager is referenced through Advice instead, and can no longer be cleared. Or is there another mechanism?

Below is the relevant code for the custom TestContextManager.

class CleanupContextManager extends TestContextManager {
    @Override
    public void beforeTestMethod( Object testInstance, Method testMethod ) throws Exception {
      super.beforeTestMethod( testInstance, testMethod );
      
      // Get the bean factory after it is created
      lastBeanFactory = (AbstractBeanFactory)getTestContext().getApplicationContext().getAutowireCapableBeanFactory();
    }

    @Override
    public void afterTestMethod( Object testInstance, Method testMethod, Throwable exception ) throws Exception {
      super.afterTestMethod( testInstance, testMethod, exception );
      
      TestContext testContext = getTestContext();
      if( null != testContext )
        testContext.markApplicationContextDirty( DirtiesContext.HierarchyMode.EXHAUSTIVE );

      if( null != lastBeanFactory ) {
        // If cleaned every method, reset post processors
        lastBeanFactory.getBeanPostProcessors().clear();
        lastBeanFactory = null;
      }
    }
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2016

Juergen Hoeller commented

I guess an easy step on our end is to declare that transaction manager cache as ConcurrentReferenceHashMap, allowing for the entries to get garbage-collected. I'll do that for 4.3.2 still (to be released tomorrow) and will also backport it to 4.2.8 (scheduled for September).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 26, 2016

Will Darby commented

Thank you. I'll try it as soon as it's released.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 27, 2016

Juergen Hoeller commented

FYI, the 4.3.2 release moved to tomorrow (European morning). For the time being, there's a 4.3.2.BUILD-SNAPSHOT if you'd like to give it an early try (see http://projects.spring.io/spring-framework/ for how to obtain it)...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 19, 2016

Tim Lenz commented

Hello,

Unfortunately this change is causing NoUniqueBeanDefinitionExceptions for one of my web applications. The application has both a DataSourceTransactionManager and a HibernateTransactionManager (in different context files, with different ids of course). As soon as a transaction cache entry is cleared by the garbage collector, the getTransactionManager() method in TransactionAspectSupport returns null, so the transaction manager bean is then looked up by type, leading to the conflict. I guess I'll have to revert back to 4.3.1 in the meantime..

org.springframework.beans.factory.NoUniqueBeanDefinitionException: No qualifying bean of type [org.springframework.transaction.PlatformTransactionManager] is defined: expected single matching bean but found 2: dataSourceTransactionManager,hibernateTxManager
at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean(DefaultListableBeanFactory.java:368)
at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean(DefaultListableBeanFactory.java:334)
at org.springframework.transaction.interceptor.TransactionAspectSupport.determineTransactionManager(TransactionAspectSupport.java:366)
at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:270)
...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 19, 2016

Sam Brannen commented

Tim Lenz, would you mind opening a new bug report (referencing this one as a regression) since this issue is already closed?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 19, 2016

Juergen Hoeller commented

Indeed, a new bug report would be appreciated. I guess we'll have to differentiate between explicitly configured and implicitly resolved transaction managers there...

Also, this is reason enough to undo this change for 4.2.8. I'd rather avoid any such side effects in that branch.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 19, 2016

Tim Lenz commented

Okay I made a new issue, #19177.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.