Spring AbstractPlatformTransactionManager.getTransaction do not clear synchronizations&cleanup transaction when catch RuntimeException

Affects: spring-tx 4.3 and later

---

hello , maybe I found one problem. In my program, Logger is not the standard Logger, will throw IllegalStateException when invoke logger.trace().And in AbstractPlatformTransactionManager, some method invoke logger.trace() so when I invoke method with @Transactional annotation, will throw exception like below, but in catch code , do not end transaction, still have transaction connection. so the same thread do other things still in transaction and reuse transaction connection,

java.lang.IllegalStateException: Unknown Priority TRACE
    at org.apache.log4j.Category.priorityToLevelInt(Category.java:326)
    at org.apache.log4j.Category.log(Category.java:296)
    at com.doudou.common.logging.spi.log4j.Log4jLogger.trace(Log4jLogger.java:106)
    at org.springframework.transaction.support.TransactionSynchronizationManager.initSynchronization(TransactionSynchronizationManager.java:272)
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.prepareSynchronization(AbstractPlatformTransactionManager.java:546)
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.handleExistingTransaction(AbstractPlatformTransactionManager.java:434)
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.getTransaction(AbstractPlatformTransactionManager.java:353)
    at org.springframework.transaction.interceptor.TransactionAspectSupport.createTransactionIfNecessary(TransactionAspectSupport.java:461)
    at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:277)
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:673)
    at com.doudou.meta.service.table.impl.MetaTableServiceImpl$$EnhancerBySpringCGLIB$$b5c28abd.addTableIgnoreIfExists(<generated>)

and in this scenario, I read the spring-tx code

    public static void initSynchronization() throws IllegalStateException {
        if (isSynchronizationActive()) {
            throw new IllegalStateException("Cannot activate transaction synchronization - already active");
        }
                 // here throw exception
        logger.trace("Initializing transaction synchronization");
        synchronizations.set(new LinkedHashSet<TransactionSynchronization>());
    }

but in catch code, only do resume ,do not clear synchronizations & transaction connection

            try {
                boolean newSynchronization = (getTransactionSynchronization() != SYNCHRONIZATION_NEVER);
                DefaultTransactionStatus status = newTransactionStatus(
                        definition, transaction, true, newSynchronization, debugEnabled, suspendedResources);
                doBegin(transaction, definition);
                prepareSynchronization(status, definition);
                return status;
            }
            catch (RuntimeException ex) {
                                 // here only do resume
                resume(null, suspendedResources);
                throw ex;
            }

maybe do like below is better:

            DefaultTransactionStatus status = null;
            try {
                boolean newSynchronization = (getTransactionSynchronization() != SYNCHRONIZATION_NEVER);
                status = newTransactionStatus(
                        definition, transaction, true, newSynchronization, debugEnabled, suspendedResources);
                doBegin(transaction, definition);
                prepareSynchronization(status, definition);
                return status;
            }
            catch (RuntimeException ex) {
                if (status != null) {
                    cleanupAfterCompletion(status);
                }
                throw ex;
            }

I see cleanupAfterCompletion will do 3 things 1. clear synchronizations 2. reset connection unbind resource. this is importent

    @Override
    protected void doCleanupAfterCompletion(Object transaction) {
        DataSourceTransactionObject txObject = (DataSourceTransactionObject) transaction;

        // Remove the connection holder from the thread, if exposed.
        if (txObject.isNewConnectionHolder()) {
            TransactionSynchronizationManager.unbindResource(this.dataSource);
        }

        // Reset connection.
        Connection con = txObject.getConnectionHolder().getConnection();
        try {
            if (txObject.isMustRestoreAutoCommit()) {
                con.setAutoCommit(true);
            }
            DataSourceUtils.resetConnectionAfterTransaction(con, txObject.getPreviousIsolationLevel());
        }
        catch (Throwable ex) {
            logger.debug("Could not reset JDBC Connection after transaction", ex);
        }

        if (txObject.isNewConnectionHolder()) {
            if (logger.isDebugEnabled()) {
                logger.debug("Releasing JDBC Connection [" + con + "] after transaction");
            }
            DataSourceUtils.releaseConnection(con, this.dataSource);
        }

        txObject.getConnectionHolder().clear();
    }

Comment From: ZZZKROSS

😯

Comment From: rstoyanchev

The given logger.trace was removed in 22686fbfa1bbe36101c608c494d54a97ebb653d0, so it is not present in the currently supported branches 5.3.x and 6.0.x.

Comment From: doudouOUC

OK. I think when catch exception, should trigger doCleanupAfterCompletion to reset connection