Repository: tomee Updated Branches: refs/heads/tomee-1.7.x 2b07bcd18 -> f5519507e
TOMEE-1860 DBCP pool can lead to a NPE for ManagedConnection closing a shared connection Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/f5519507 Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/f5519507 Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/f5519507 Branch: refs/heads/tomee-1.7.x Commit: f5519507e96c751f15d9a0151ab7082e883fc9e8 Parents: 2b07bcd Author: Romain manni-Bucau <[email protected]> Authored: Tue Jul 5 20:10:49 2016 +0200 Committer: Romain manni-Bucau <[email protected]> Committed: Tue Jul 5 20:10:49 2016 +0200 ---------------------------------------------------------------------- .../jdbc/dbcp/BasicManagedDataSource.java | 29 +++++ .../resource/jdbc/DbcpNPEXAConnectionTest.java | 112 +++++++++++++++++++ 2 files changed, 141 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/f5519507/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java index ad32dbe..841e898 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java @@ -18,6 +18,9 @@ package org.apache.openejb.resource.jdbc.dbcp; import org.apache.commons.dbcp.ConnectionFactory; +import org.apache.commons.dbcp.managed.ManagedConnection; +import org.apache.commons.dbcp.managed.ManagedDataSource; +import org.apache.commons.dbcp.managed.TransactionRegistry; import org.apache.openejb.OpenEJB; import org.apache.openejb.cipher.PasswordCipher; import org.apache.openejb.cipher.PasswordCipherFactory; @@ -29,6 +32,7 @@ import org.apache.openejb.resource.jdbc.pool.XADataSourceResource; import javax.sql.DataSource; import java.io.File; +import java.sql.Connection; import java.sql.SQLException; import java.sql.SQLFeatureNotSupportedException; import java.util.Properties; @@ -79,6 +83,31 @@ public class BasicManagedDataSource extends org.apache.commons.dbcp.managed.Basi return super.createConnectionFactory(); } + @Override + protected void createDataSourceInstance() throws SQLException { + final TransactionRegistry transactionRegistry = getTransactionRegistry(); + if (transactionRegistry == null) { + throw new IllegalStateException("TransactionRegistry has not been set"); + } + if (connectionPool == null) { + throw new IllegalStateException("Pool has not been set"); + } + super.dataSource = new ManagedDataSource(connectionPool, transactionRegistry) { + @Override + public Connection getConnection() throws SQLException { + return new ManagedConnection(connectionPool, transactionRegistry, isAccessToUnderlyingConnectionAllowed()) { + @Override + public void close() throws SQLException { + if (getDelegateInternal() == null) { + return; + } + super.close(); + } + }; + } + }; + } + private void setJndiXaDataSource(final String xaDataSource) { setXaDataSourceInstance( // proxy cause we don't know if this datasource was created before or not the delegate XADataSourceResource.proxy(getDriverClassLoader() != null ? getDriverClassLoader() : Thread.currentThread().getContextClassLoader(), xaDataSource)); http://git-wip-us.apache.org/repos/asf/tomee/blob/f5519507/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/DbcpNPEXAConnectionTest.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/DbcpNPEXAConnectionTest.java b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/DbcpNPEXAConnectionTest.java new file mode 100644 index 0000000..8aaaed6 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/DbcpNPEXAConnectionTest.java @@ -0,0 +1,112 @@ +/** + * 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.openejb.resource.jdbc; + +import org.apache.commons.pool.impl.GenericObjectPool; +import org.apache.openejb.jee.EjbJar; +import org.apache.openejb.junit.ApplicationComposer; +import org.apache.openejb.resource.jdbc.pool.DefaultDataSourceCreator; +import org.apache.openejb.testing.Classes; +import org.apache.openejb.testing.Configuration; +import org.apache.openejb.testing.Module; +import org.apache.openejb.testing.SimpleLog; +import org.apache.openejb.testng.PropertiesBuilder; +import org.apache.openejb.util.reflection.Reflections; +import org.hsqldb.jdbc.pool.JDBCXADataSource; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.annotation.Resource; +import javax.ejb.EJB; +import javax.ejb.Singleton; +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.Properties; + +import static org.junit.Assert.assertEquals; + +@SimpleLog +@RunWith(ApplicationComposer.class) +public class DbcpNPEXAConnectionTest { + @Resource(name = "xadb") + private DataSource ds; + + @EJB + private AnEjb ejb; + + @Module + @Classes(AnEjb.class) + public EjbJar mandatory() { + return new EjbJar(); + } + + @Configuration + public Properties props() { + return new PropertiesBuilder() + .p("openejb.jdbc.datasource-creator", DefaultDataSourceCreator.class.getName()) + + .p("txMgr", "new://TransactionManager?type=TransactionManager") + .p("txMgr.txRecovery", "true") + .p("txMgr.logFileDir", "target/test/xa/DbcpNPEXAConnectionTest") + + // real XA datasources + .p("xa", "new://Resource?class-name=" + JDBCXADataSource.class.getName()) + .p("xa.url", "jdbc:hsqldb:mem:dbcpxa") + .p("xa.user", "sa") + .p("xa.password", "") + .p("xa.SkipImplicitAttributes", "true") + .p("xa.SkipPropertiesFallback", "true") // otherwise goes to connection properties + + .p("xadb", "new://Resource?type=DataSource") + .p("xadb.xaDataSource", "xa") + .p("xadb.JtaManaged", "true") + .p("xadb.MaxIdle", "25") + .p("xadb.MaxTotal", "25") + .p("xadb.InitialSize", "3") + + .build(); + } + + @Test + public void check() throws SQLException { + ejb.newConn().close(); // no NPE + final GenericObjectPool pool = GenericObjectPool.class.cast(Reflections.get(ds, "connectionPool")); + assertEquals(0, pool.getNumActive()); + } + + @Singleton + public static class AnEjb { + @Resource(name = "xadb") + private DataSource ds; + + public Connection newConn() throws SQLException { + ds.getConnection().close(); // first connection is not "shared" so closes correctly + return use(ds.getConnection()); // this one is shared so delegate will be null and close outside JTA will fail + } + } + + private static Connection use(final Connection connection) { + try { + connection.getMetaData(); + } catch (final SQLException e) { + Assert.fail(e.getMessage()); + } + return connection; + } +}
