DBCP-515 Added support for preventing attempt to enlist a Synchroniza…

…tion during afterCompletion
apache · Jul 25, 2018 · 78e5cb7 · 78e5cb7
1 parent d7969ac
commit 78e5cb7
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 5 deletions.
diff --git a/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java b/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java
@@ -203,7 +203,7 @@ public void close() throws SQLException {
      */
     protected class CompletionListener implements TransactionContextListener {
         @Override
-        public void afterCompletion(final TransactionContext completedContext, final boolean commited) {
+        public void afterCompletion(final TransactionContext completedContext) {
             if (completedContext == transactionContext) {
                 transactionComplete();
             }

diff --git a/src/main/java/org/apache/commons/dbcp2/managed/TransactionContext.java b/src/main/java/org/apache/commons/dbcp2/managed/TransactionContext.java
@@ -129,6 +129,10 @@ public void setSharedConnection(final Connection sharedConnection) throws SQLExc
      *             if a problem occurs adding the listener to the transaction
      */
     public void addTransactionContextListener(final TransactionContextListener listener) throws SQLException {
+        if (!isActive()) {
+            listener.afterCompletion(TransactionContext.this);
+            return;
+        }
         try {
             final Synchronization s = new Synchronization() {
                 @Override
@@ -138,7 +142,7 @@ public void beforeCompletion() {
 
                 @Override
                 public void afterCompletion(final int status) {
-                    listener.afterCompletion(TransactionContext.this, status == Status.STATUS_COMMITTED);
+                    listener.afterCompletion(TransactionContext.this);
                 }
             };
             if (transactionSynchronizationRegistry != null) {

diff --git a/src/main/java/org/apache/commons/dbcp2/managed/TransactionContextListener.java b/src/main/java/org/apache/commons/dbcp2/managed/TransactionContextListener.java
@@ -28,8 +28,6 @@ public interface TransactionContextListener {
      *
      * @param transactionContext
      *            the transaction context that completed
-     * @param commited
-     *            true if the transaction committed; false otherwise
      */
-    void afterCompletion(TransactionContext transactionContext, boolean commited);
+    void afterCompletion(TransactionContext transactionContext);
 }
diff --git a/src/test/java/org/apache/commons/dbcp2/managed/TestManagedDataSourceInTx.java b/src/test/java/org/apache/commons/dbcp2/managed/TestManagedDataSourceInTx.java
@@ -36,6 +36,7 @@
 import java.sql.ResultSet;
 import java.sql.PreparedStatement;
 
+import javax.transaction.Synchronization;
 import javax.transaction.Transaction;
 
 /**
@@ -59,6 +60,36 @@ public void tearDown() throws Exception {
         super.tearDown();
     }
 
+    @Test
+    public void testGetConnectionInAfterCompletion() throws Exception {
+
+        final DelegatingConnection<?> connection = (DelegatingConnection<?>) newConnection();
+        // Don't close so we can check it for warnings in afterCompletion
+        transactionManager.getTransaction().registerSynchronization(new Synchronization() {
+            @Override
+            public void beforeCompletion() {
+
+            }
+
+            @Override
+            public void afterCompletion(int i) {
+                try {
+                    Connection connection1 = ds.getConnection();
+                    try {
+                        connection1.getWarnings();
+                        fail("Could operate on closed connection");
+                    } catch (SQLException e) {
+                        // This is expected
+                    }
+                } catch (SQLException e) {
+                    fail("Should have been able to get connection");
+                }
+            }
+        });
+        connection.close();
+        transactionManager.commit();
+    }
+
     /**
      * @see #testSharedConnection()
      */