TOMEE-1643 ensure to close the XAConnection to not leak connection or break the pool
Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/74bd06dd Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/74bd06dd Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/74bd06dd Branch: refs/heads/tomee-7.0.0-M1 Commit: 74bd06dd53580b9f4ccc5f3e70bff2671b7a3ff9 Parents: 234a109 Author: Romain Manni-Bucau <[email protected]> Authored: Thu Oct 22 12:29:59 2015 +0200 Committer: Romain Manni-Bucau <[email protected]> Committed: Thu Oct 22 12:29:59 2015 +0200 ---------------------------------------------------------------------- .../jdbc/managed/local/ManagedConnection.java | 50 ++++++++++++-------- .../tomee/jdbc/TomcatXADataSourceTest.java | 39 +++++++++++++-- 2 files changed, 66 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/74bd06dd/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java index 83ce588..91dfb61 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java @@ -20,12 +20,6 @@ package org.apache.openejb.resource.jdbc.managed.local; import org.apache.openejb.util.LogCategory; import org.apache.openejb.util.Logger; -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.sql.Connection; -import java.sql.SQLException; -import java.sql.Wrapper; import javax.sql.CommonDataSource; import javax.sql.DataSource; import javax.sql.XAConnection; @@ -38,6 +32,12 @@ import javax.transaction.Transaction; import javax.transaction.TransactionManager; import javax.transaction.TransactionSynchronizationRegistry; import javax.transaction.xa.XAResource; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.sql.Connection; +import java.sql.SQLException; +import java.sql.Wrapper; public class ManagedConnection implements InvocationHandler { private final TransactionManager transactionManager; @@ -96,6 +96,14 @@ public class ManagedConnection implements InvocationHandler { // shouldn't be used without a transaction but if so just delegate to the actual connection if (transaction == null) { + if ("close".equals(mtdName)) { + if (delegate == null) { // no need to get a connection + return null; + } + + closeConnection(xaConnection, delegate); + return null; + } if (delegate == null) { newConnection(); } @@ -127,7 +135,7 @@ public class ManagedConnection implements InvocationHandler { throw new SQLException("Unable to enlist connection the transaction", e); } - transaction.registerSynchronization(new ClosingSynchronization(delegate)); + transaction.registerSynchronization(new ClosingSynchronization(xaConnection, delegate)); try { setAutoCommit(false); @@ -220,20 +228,12 @@ public class ManagedConnection implements InvocationHandler { return new SQLException("can't call " + mtdName + " when the connection is JtaManaged"); } - private static void close(final Connection connection) { - try { - if (!connection.isClosed()) { - connection.close(); - } - } catch (final SQLException e) { - // no-op - } - } - private static class ClosingSynchronization implements Synchronization { + private final XAConnection xaConnection; private final Connection connection; - public ClosingSynchronization(final Connection delegate) { + public ClosingSynchronization(final XAConnection xaConnection, final Connection delegate) { + this.xaConnection = xaConnection; this.connection = delegate; } @@ -244,7 +244,19 @@ public class ManagedConnection implements InvocationHandler { @Override public void afterCompletion(final int status) { - close(connection); + closeConnection(xaConnection, connection); + } + } + + private static void closeConnection(final XAConnection xaConnection, final Connection connection) { + try { + if (xaConnection != null) { // handles the underlying connection + xaConnection.close(); + } else if (connection != null && !connection.isClosed()) { + connection.close(); + } + } catch (final SQLException e) { + // no-op } } http://git-wip-us.apache.org/repos/asf/tomee/blob/74bd06dd/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatXADataSourceTest.java ---------------------------------------------------------------------- diff --git a/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatXADataSourceTest.java b/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatXADataSourceTest.java index ecffcb9..5863384 100644 --- a/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatXADataSourceTest.java +++ b/tomee/tomee-jdbc/src/test/java/org/apache/tomee/jdbc/TomcatXADataSourceTest.java @@ -18,6 +18,7 @@ package org.apache.tomee.jdbc; import org.apache.openejb.jee.EjbJar; import org.apache.openejb.junit.ApplicationComposer; +import org.apache.openejb.resource.jdbc.managed.local.ManagedDataSource; import org.apache.openejb.testing.Configuration; import org.apache.openejb.testing.Module; import org.apache.openejb.testng.PropertiesBuilder; @@ -30,9 +31,12 @@ import javax.annotation.Resource; import javax.sql.DataSource; import java.sql.Connection; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Collection; import java.util.Properties; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; @@ -66,6 +70,9 @@ public class TomcatXADataSourceTest { .p("xadb", "new://Resource?type=DataSource") .p("xadb.xaDataSource", "xa") .p("xadb.JtaManaged", "true") + .p("xadb.MaxIdle", "25") + .p("xadb.MaxActive", "25") + .p("xadb.InitialSize", "3") .build(); } @@ -73,10 +80,34 @@ public class TomcatXADataSourceTest { @Test public void check() throws SQLException { assertNotNull(ds); - final Connection c = ds.getConnection(); - assertNotNull(c); - assertThat(c.getMetaData().getConnection(), instanceOf(JDBCXAConnectionWrapper.class)); - c.close(); + final TomEEDataSourceCreator.TomEEDataSource tds = TomEEDataSourceCreator.TomEEDataSource.class.cast(ManagedDataSource.class.cast(ds).getDelegate()); + assertEquals(3, tds.getIdle()); // InitSize + + try (final Connection c = ds.getConnection()) { + assertNotNull(c); + + final Connection connection = c.getMetaData().getConnection(); // just to do something and force the connection init + assertThat(connection, instanceOf(JDBCXAConnectionWrapper.class)); + } // here we close the connection so we are back in the initial state + + assertEquals(0, tds.getActive()); + assertEquals(3, tds.getIdle()); + + for (int it = 0; it < 5; it++) { // ensures it always works and not only the first time + final Collection<Connection> connections = new ArrayList<>(25); + for (int i = 0; i < 25; i++) { + final Connection connection = ds.getConnection(); + connections.add(connection); + connection.getMetaData(); // trigger connection retrieving otherwise nothing is done (pool is not used) + } + assertEquals(25, tds.getActive()); + assertEquals(0, tds.getIdle()); + for (final Connection toClose : connections) { + toClose.close(); + } + assertEquals(0, tds.getActive()); + assertEquals(25, tds.getIdle()); + } } }
