Repository: commons-dbcp Updated Branches: refs/heads/master 9a9be4855 -> 377dd0a46
[DBCP-491] Ensure DBCP ConnectionListener can deal with transaction managers which invoke rollback in a separate thread. Applied modified patch from Zheng Feng. Project: http://git-wip-us.apache.org/repos/asf/commons-dbcp/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-dbcp/commit/377dd0a4 Tree: http://git-wip-us.apache.org/repos/asf/commons-dbcp/tree/377dd0a4 Diff: http://git-wip-us.apache.org/repos/asf/commons-dbcp/diff/377dd0a4 Branch: refs/heads/master Commit: 377dd0a46189c53b9af1f89c7e4e9e1bc7698646 Parents: 9a9be48 Author: Gary Gregory <garydgreg...@gmail.com> Authored: Tue May 29 15:04:45 2018 -0600 Committer: Gary Gregory <garydgreg...@gmail.com> Committed: Tue May 29 15:04:45 2018 -0600 ---------------------------------------------------------------------- .gitignore | 3 +- pom.xml | 66 ++++++ src/changes/changes.xml | 3 + .../dbcp2/managed/ManagedConnection.java | 6 +- .../dbcp2/managed/TransactionContext.java | 13 ++ .../dbcp2/managed/TransactionRegistry.java | 7 +- .../managed/TestConnectionWithNarayana.java | 231 +++++++++++++++++++ 7 files changed, 320 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/377dd0a4/.gitignore ---------------------------------------------------------------------- diff --git a/.gitignore b/.gitignore index fc55ebb..3010606 100644 --- a/.gitignore +++ b/.gitignore @@ -13,4 +13,5 @@ site-content /.classpath /.pmd /.project -/.settings \ No newline at end of file +/.settings +/ObjectStore/ http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/377dd0a4/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index 65488de..f73d931 100644 --- a/pom.xml +++ b/pom.xml @@ -215,6 +215,42 @@ </exclusion> </exclusions> </dependency> + <dependency> + <groupId>com.h2database</groupId> + <artifactId>h2</artifactId> + <version>1.4.197</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.jboss.narayana.jta</groupId> + <artifactId>narayana-jta</artifactId> + <version>5.8.2.Final</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.jboss.spec.javax.transaction</groupId> + <artifactId>jboss-transaction-api_1.2_spec</artifactId> + <version>1.0.0.Final</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.jboss</groupId> + <artifactId>jboss-transaction-spi</artifactId> + <version>7.6.0.Final</version> + <exclusions> + <exclusion> + <groupId>org.jboss.logging</groupId> + <artifactId>jboss-logging-spi</artifactId> + </exclusion> + </exclusions> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.jboss.logging</groupId> + <artifactId>jboss-logging</artifactId> + <version>3.1.4.GA</version> + <scope>test</scope> + </dependency> </dependencies> <properties> @@ -417,4 +453,34 @@ </plugin> </plugins> </reporting> + <profiles> + <profile> + <id>jdk7</id> + <activation> + <jdk>(,1.7]</jdk> + </activation> + <build> + <plugins> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-surefire-plugin</artifactId> + <configuration> + <systemPropertyVariables> + <!-- Ensure that logging messages can be inspected --> + <org.apache.commons.logging.Log>org.apache.commons.dbcp2.StackMessageLog</org.apache.commons.logging.Log> + </systemPropertyVariables> + <reportFormat>plain</reportFormat> + <excludes> + <!-- Test support files --> + <exclude>**/Tester*.java</exclude> + <!-- Exclude nested classes which Surefire cannot handle --> + <exclude>**/Test*$*.java</exclude> + <exclude>**/*Narayana*.java</exclude> + </excludes> + </configuration> + </plugin> + </plugins> + </build> + </profile> + </profiles> </project> http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/377dd0a4/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 9f36c1e..cea3bab 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -67,6 +67,9 @@ The <action> type attribute can be add,update,fix,remove. <action dev="ggregory" type="update" issue="DBCP-492" due-to="Gary Gregory"> Drop Ant build. </action> + <action dev="ggregory" type="update" issue="DBCP-491" due-to="Zheng Feng, Gary Gregory"> + Ensure DBCP ConnectionListener can deal with transaction managers which invoke rollback in a separate thread. + </action> </release> <release version="2.3.0" date="2018-05-12" description="This is a minor release, including bug fixes and enhancements."> <action dev="pschumacher" type="fix" issue="DBCP-476" due-to="Gary Evesson, Richard Cordova"> http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/377dd0a4/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java b/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java index c50327f..d5154f7 100644 --- a/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java +++ b/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java @@ -71,7 +71,7 @@ public class ManagedConnection<C extends Connection> extends DelegatingConnectio private void updateTransactionStatus() throws SQLException { // if there is a is an active transaction context, assure the transaction context hasn't changed - if (transactionContext != null) { + if (transactionContext != null && !transactionContext.isTransactionComplete()) { if (transactionContext.isActive()) { if (transactionContext != transactionRegistry.getActiveTransactionContext()) { throw new SQLException("Connection can not be used while enlisted in another transaction"); @@ -172,7 +172,7 @@ public class ManagedConnection<C extends Connection> extends DelegatingConnectio // be modified by the transactionComplete method which could run // in the different thread with the transaction calling back. lock.lock(); - if (transactionContext == null) { + if (transactionContext == null || transactionContext.isTransactionComplete()) { super.close(); } } finally { @@ -198,7 +198,7 @@ public class ManagedConnection<C extends Connection> extends DelegatingConnectio protected void transactionComplete() { lock.lock(); - transactionContext = null; + transactionContext.completeTransaction(); lock.unlock(); // If we were using a shared connection, clear the reference now that http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/377dd0a4/src/main/java/org/apache/commons/dbcp2/managed/TransactionContext.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/dbcp2/managed/TransactionContext.java b/src/main/java/org/apache/commons/dbcp2/managed/TransactionContext.java index 9e472cf..989f4dd 100644 --- a/src/main/java/org/apache/commons/dbcp2/managed/TransactionContext.java +++ b/src/main/java/org/apache/commons/dbcp2/managed/TransactionContext.java @@ -40,6 +40,7 @@ public class TransactionContext { private final TransactionRegistry transactionRegistry; private final WeakReference<Transaction> transactionRef; private Connection sharedConnection; + private boolean transactionComplete; /** * Creates a TransactionContext for the specified Transaction and TransactionRegistry. The @@ -59,6 +60,7 @@ public class TransactionContext { } this.transactionRegistry = transactionRegistry; this.transactionRef = new WeakReference<>(transaction); + this.transactionComplete = false; } /** @@ -93,6 +95,9 @@ public class TransactionContext { if ( !transaction.enlistResource(xaResource) ) { throw new SQLException("Unable to enlist connection in transaction: enlistResource returns 'false'."); } + } catch (final IllegalStateException e) { + // This can happen if the transaction is already timed out + throw new SQLException("Unable to enlist connection in the transaction", e); } catch (final RollbackException e) { // transaction was rolled back... proceed as if there never was a transaction } catch (final SystemException e) { @@ -153,4 +158,12 @@ public class TransactionContext { } return transaction; } + + public void completeTransaction() { + this.transactionComplete = true; + } + + public boolean isTransactionComplete() { + return this.transactionComplete; + } } http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/377dd0a4/src/main/java/org/apache/commons/dbcp2/managed/TransactionRegistry.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/dbcp2/managed/TransactionRegistry.java b/src/main/java/org/apache/commons/dbcp2/managed/TransactionRegistry.java index ff043dd..61c55eb 100644 --- a/src/main/java/org/apache/commons/dbcp2/managed/TransactionRegistry.java +++ b/src/main/java/org/apache/commons/dbcp2/managed/TransactionRegistry.java @@ -105,11 +105,8 @@ public class TransactionRegistry { return null; } - // is it active - final int status = transaction.getStatus(); - if (status != Status.STATUS_ACTIVE && status != Status.STATUS_MARKED_ROLLBACK) { - return null; - } + // This is the transaction on the thread so no need to check it's status - we should try to use it and + // fail later based on the subsequent status } catch (final SystemException e) { throw new SQLException("Unable to determine current transaction ", e); } http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/377dd0a4/src/test/java/org/apache/commons/dbcp2/managed/TestConnectionWithNarayana.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/dbcp2/managed/TestConnectionWithNarayana.java b/src/test/java/org/apache/commons/dbcp2/managed/TestConnectionWithNarayana.java new file mode 100644 index 0000000..a17c5b6 --- /dev/null +++ b/src/test/java/org/apache/commons/dbcp2/managed/TestConnectionWithNarayana.java @@ -0,0 +1,231 @@ +/** + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.dbcp2.managed; + +import static org.junit.Assert.fail; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Timestamp; + +import javax.transaction.RollbackException; +import javax.transaction.Status; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionManagerImple; +import com.arjuna.ats.jta.common.jtaPropertyManager; + +/** + * Requires Java 8. + */ +public class TestConnectionWithNarayana { + private static final String CREATE_STMT = "CREATE TABLE TEST_DATA (KEY VARCHAR(100), ID BIGINT, VALUE DOUBLE PRECISION, INFO TEXT, TS TIMESTAMP)"; + private static final String INSERT_STMT = "INSERT INTO TEST_DATA (KEY, ID, VALUE, INFO, TS) VALUES (?,?,?,?,?)"; + private static final String SELECT_STMT = "SELECT KEY, ID, VALUE, INFO, TS FROM TEST_DATA LIMIT 1"; + private static String PAYLOAD; + private static final String DROP_STMT = "DROP TABLE TEST_DATA"; + + static { + StringBuffer sb = new StringBuffer(); + sb.append("Start"); + sb.append("payload"); + for (int i = 0; i < 10000; i++) { + sb.append("..."); + sb.append(String.valueOf(i)); + } + sb.append("End"); + sb.append("payload"); + + PAYLOAD = sb.toString(); + } + + private BasicManagedDataSource mds; + + @Before + public void setUp() throws Exception { + jtaPropertyManager.getJTAEnvironmentBean().setLastResourceOptimisationInterfaceClassName( + "org.apache.commons.dbcp2.managed.LocalXAConnectionFactory$LocalXAResource"); + mds = new BasicManagedDataSource(); + mds.setTransactionManager(new TransactionManagerImple()); + mds.setDriverClassName("org.h2.Driver"); + mds.setUrl("jdbc:h2:mem:test;DB_CLOSE_DELAY=-1"); + + mds.setMaxTotal(80); + mds.setMinIdle(0); + mds.setMaxIdle(80); + mds.setMinEvictableIdleTimeMillis(10000); + mds.setTimeBetweenEvictionRunsMillis(10000); + mds.setLogAbandoned(true); + mds.setMaxWaitMillis(2000); + mds.setRemoveAbandonedOnMaintenance(true); + mds.setRemoveAbandonedOnBorrow(true); + + mds.setRemoveAbandonedTimeout(10); + mds.setLogExpiredConnections(true); + mds.setLifo(false); + + Connection conn = mds.getConnection(); + PreparedStatement ps = conn.prepareStatement(CREATE_STMT); + ps.execute(); + ps.close(); + conn.close(); + } + + @Test + public void testRepeatedGetConnectionInTimeout() throws Exception { + mds.getTransactionManager().setTransactionTimeout(1); + mds.getTransactionManager().begin(); + + try { + do { + Thread.sleep(1000); + } while (mds.getTransactionManager().getTransaction().getStatus() != Status.STATUS_ROLLEDBACK); + // Let the reaper do it's thing + Thread.sleep(1000); + try (Connection conn = mds.getConnection()) { + fail("Should not get the connection 1"); + } catch (SQLException e) { + if (!e.getCause().getClass().equals(IllegalStateException.class)) { + throw e; + } + try (Connection conn = mds.getConnection()) { + fail("Should not get connection 2"); + } catch (SQLException e2) { + if (!e2.getCause().getClass().equals(IllegalStateException.class)) { + throw e2; + } + } + } + } finally { + mds.getTransactionManager().rollback(); + } + Assert.assertEquals(0, mds.getNumActive()); + } + + @Test + public void testConnectionCommitAfterTimeout() throws Exception { + mds.getTransactionManager().setTransactionTimeout(1); + mds.getTransactionManager().begin(); + try (Connection conn = mds.getConnection();) { + do { + Thread.sleep(1000); + } while (mds.getTransactionManager().getTransaction().getStatus() != Status.STATUS_ROLLEDBACK); + // Let the reaper do it's thing + Thread.sleep(1000); + try { + conn.commit(); + fail("Should not work after timeout"); + } catch (SQLException e) { + // Expected + Assert.assertEquals("Commit can not be set while enrolled in a transaction", e.getMessage()); + } + mds.getTransactionManager().rollback(); + } + + Assert.assertEquals(0, mds.getNumActive()); + } + + @Test + public void testConnectionInTimeout() throws Exception { + Connection conn = null; + PreparedStatement ps = null; + for (int i = 0; i < 5; i++) { + try { + mds.getTransactionManager().setTransactionTimeout(1); + mds.getTransactionManager().begin(); + + conn = mds.getConnection(); + ps = conn.prepareStatement(INSERT_STMT); + ps.setString(1, Thread.currentThread().getName()); + ps.setLong(2, i); + ps.setDouble(3, new java.util.Random().nextDouble()); + ps.setString(4, PAYLOAD); + ps.setTimestamp(5, new Timestamp(System.currentTimeMillis())); + ps.execute(); + + int n = 0; + do { + if (mds.getTransactionManager().getTransaction().getStatus() != Status.STATUS_ACTIVE) { + n++; + } + + Connection c = null; + PreparedStatement ps2 = null; + ResultSet rs = null; + try { + c = mds.getConnection(); + ps2 = c.prepareStatement(SELECT_STMT); + rs = ps2.executeQuery(); + } finally { + if (rs != null) + rs.close(); + if (ps2 != null) + ps2.close(); + if (c != null) + c.close(); + } + } while (n < 2); + + ps.close(); + ps = null; + conn.close(); + conn = null; + + try { + mds.getTransactionManager().commit(); + fail("Should not have been able to commit"); + } catch (RollbackException e) { + // this is expected + if (mds.getTransactionManager().getTransaction() != null) { + // Need to pop it off the thread if a background thread rolled the transaction back + mds.getTransactionManager().rollback(); + } + } + } catch (Exception e) { + if (mds.getTransactionManager().getTransaction() != null) { + // Need to pop it off the thread if a background thread rolled the transaction back + mds.getTransactionManager().rollback(); + } + } finally { + if (ps != null) + ps.close(); + if (conn != null) + conn.close(); + } + Assert.assertEquals(0, mds.getNumActive()); + } + } + + @After + public void tearDown() throws Exception { + Connection conn = mds.getConnection(); + PreparedStatement ps = conn.prepareStatement(DROP_STMT); + ps.execute(); + ps.close(); + conn.close(); + if (mds != null) { + mds.close(); + } + } +} \ No newline at end of file