[commons-dbcp] branch master updated: [DBCP-564] Fix BasicManagedDataSource leak of connections opened after transaction is rollback-only (#39)

[gg...@apache.org]

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 <fg...@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());
+    }
 }