This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git
The following commit(s) were added to refs/heads/master by this push: new 1474ba3 [DBCP-564] Fix BasicManagedDataSource leak of connections opened after transaction is rollback-only (#39) 1474ba3 is described below commit 1474ba327798ebfabbcf5ab1db01330851f3377c Author: Florent Guillaume <f...@nuxeo.com> AuthorDate: Thu May 14 16:19:21 2020 +0200 [DBCP-564] Fix BasicManagedDataSource leak of connections opened after transaction is rollback-only (#39) --- .../dbcp2/managed/BasicManagedDataSource.java | 2 +- .../dbcp2/managed/LocalXAConnectionFactory.java | 20 ++++++++++++++++- .../dbcp2/managed/TestBasicManagedDataSource.java | 25 +++++++++++++++++++++- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/dbcp2/managed/BasicManagedDataSource.java b/src/main/java/org/apache/commons/dbcp2/managed/BasicManagedDataSource.java index 9e312ce..11dcbcd 100644 --- a/src/main/java/org/apache/commons/dbcp2/managed/BasicManagedDataSource.java +++ b/src/main/java/org/apache/commons/dbcp2/managed/BasicManagedDataSource.java @@ -174,7 +174,7 @@ public class BasicManagedDataSource extends BasicDataSource { if (xaDataSource == null) { final ConnectionFactory connectionFactory = super.createConnectionFactory(); final XAConnectionFactory xaConnectionFactory = new LocalXAConnectionFactory(getTransactionManager(), - connectionFactory); + getTransactionSynchronizationRegistry(), connectionFactory); transactionRegistry = xaConnectionFactory.getTransactionRegistry(); return xaConnectionFactory; } diff --git a/src/main/java/org/apache/commons/dbcp2/managed/LocalXAConnectionFactory.java b/src/main/java/org/apache/commons/dbcp2/managed/LocalXAConnectionFactory.java index 3fa6636..50e9f91 100644 --- a/src/main/java/org/apache/commons/dbcp2/managed/LocalXAConnectionFactory.java +++ b/src/main/java/org/apache/commons/dbcp2/managed/LocalXAConnectionFactory.java @@ -20,6 +20,7 @@ package org.apache.commons.dbcp2.managed; import org.apache.commons.dbcp2.ConnectionFactory; import javax.transaction.TransactionManager; +import javax.transaction.TransactionSynchronizationRegistry; import javax.transaction.xa.XAException; import javax.transaction.xa.XAResource; import javax.transaction.xa.Xid; @@ -310,17 +311,34 @@ public class LocalXAConnectionFactory implements XAConnectionFactory { * * @param transactionManager * the transaction manager in which connections will be enlisted + * @param transactionSynchronizationRegistry + * the optional TSR to register synchronizations with * @param connectionFactory * the connection factory from which connections will be retrieved */ public LocalXAConnectionFactory(final TransactionManager transactionManager, + final TransactionSynchronizationRegistry transactionSynchronizationRegistry, final ConnectionFactory connectionFactory) { Objects.requireNonNull(transactionManager, "transactionManager is null"); Objects.requireNonNull(connectionFactory, "connectionFactory is null"); - this.transactionRegistry = new TransactionRegistry(transactionManager); + this.transactionRegistry = new TransactionRegistry(transactionManager, transactionSynchronizationRegistry); this.connectionFactory = connectionFactory; } + /** + * Creates an LocalXAConnectionFactory which uses the specified connection factory to create database connections. + * The connections are enlisted into transactions using the specified transaction manager. + * + * @param transactionManager + * the transaction manager in which connections will be enlisted + * @param connectionFactory + * the connection factory from which connections will be retrieved + */ + public LocalXAConnectionFactory(final TransactionManager transactionManager, + final ConnectionFactory connectionFactory) { + this(transactionManager, null, connectionFactory); + } + @Override public Connection createConnection() throws SQLException { // create a new connection diff --git a/src/test/java/org/apache/commons/dbcp2/managed/TestBasicManagedDataSource.java b/src/test/java/org/apache/commons/dbcp2/managed/TestBasicManagedDataSource.java index d4576ef..64de034 100644 --- a/src/test/java/org/apache/commons/dbcp2/managed/TestBasicManagedDataSource.java +++ b/src/test/java/org/apache/commons/dbcp2/managed/TestBasicManagedDataSource.java @@ -48,7 +48,9 @@ public class TestBasicManagedDataSource extends TestBasicDataSource { @Override protected BasicDataSource createDataSource() throws Exception { final BasicManagedDataSource basicManagedDataSource = new BasicManagedDataSource(); - basicManagedDataSource.setTransactionManager(new TransactionManagerImpl()); + TransactionManagerImpl transactionManager = new TransactionManagerImpl(); + basicManagedDataSource.setTransactionManager(transactionManager); + basicManagedDataSource.setTransactionSynchronizationRegistry(transactionManager); return basicManagedDataSource; } @@ -219,4 +221,25 @@ public class TestBasicManagedDataSource extends TestBasicDataSource { tm.commit(); } } + + /** DBCP-564 */ + @Test + public void testSetRollbackOnlyBeforeGetConnectionDoesNotLeak() throws Exception { + final TransactionManager transactionManager = ((BasicManagedDataSource) ds).getTransactionManager(); + final int n = 3; + ds.setMaxIdle(n); + ds.setMaxTotal(n); + + for (int i = 0; i <= n; i++) { // loop n+1 times + transactionManager.begin(); + transactionManager.setRollbackOnly(); + final Connection conn = getConnection(); + assertNotNull(conn); + conn.close(); + transactionManager.rollback(); + } + + assertEquals(0, ds.getNumActive()); + assertEquals(1, ds.getNumIdle()); + } }