You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2018/05/29 21:05:02 UTC
commons-dbcp git commit: [DBCP-491] Ensure DBCP ConnectionListener
can deal with transaction managers which invoke rollback in a separate
thread. Applied modified patch from Zheng Feng.
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 <ga...@gmail.com>
Authored: Tue May 29 15:04:45 2018 -0600
Committer: Gary Gregory <ga...@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