Repository: tomee Updated Branches: refs/heads/master 52828b17e -> f4a8864bf
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/f4a8864b Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/f4a8864b Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/f4a8864b Branch: refs/heads/master Commit: f4a8864bfe935d80d16a3807dcd8182b7049a1a3 Parents: 52828b1 Author: Romain manni-Bucau <[email protected]> Authored: Tue Jul 5 12:55:29 2016 +0200 Committer: Romain manni-Bucau <[email protected]> Committed: Tue Jul 5 12:55:29 2016 +0200 ---------------------------------------------------------------------- .../jdbc/dbcp/BasicManagedDataSource.java | 33 ++++++ .../resource/jdbc/DbcpNPEXAConnectionTest.java | 114 +++++++++++++++++++ 2 files changed, 147 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/f4a8864b/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 75bdf19..6288b7f 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,11 @@ package org.apache.openejb.resource.jdbc.dbcp; import org.apache.commons.dbcp2.ConnectionFactory; +import org.apache.commons.dbcp2.PoolableConnection; +import org.apache.commons.dbcp2.PoolingDataSource; +import org.apache.commons.dbcp2.managed.ManagedConnection; +import org.apache.commons.dbcp2.managed.ManagedDataSource; +import org.apache.commons.dbcp2.managed.TransactionRegistry; import org.apache.openejb.OpenEJB; import org.apache.openejb.cipher.PasswordCipher; import org.apache.openejb.cipher.PasswordCipherFactory; @@ -34,6 +39,7 @@ import javax.sql.DataSource; import java.io.File; import java.io.ObjectStreamException; import java.io.Serializable; +import java.sql.Connection; import java.sql.SQLException; import java.sql.SQLFeatureNotSupportedException; import java.util.Properties; @@ -68,6 +74,33 @@ public class BasicManagedDataSource extends org.apache.commons.dbcp2.managed.Bas } @Override + protected DataSource createDataSourceInstance() throws SQLException { + final TransactionRegistry transactionRegistry = getTransactionRegistry(); + if (transactionRegistry == null) { + throw new IllegalStateException("TransactionRegistry has not been set"); + } + if (getConnectionPool() == null) { + throw new IllegalStateException("Pool has not been set"); + } + final PoolingDataSource<PoolableConnection> pds = new ManagedDataSource<PoolableConnection>(getConnectionPool(), transactionRegistry) { + @Override + public Connection getConnection() throws SQLException { + return new ManagedConnection<PoolableConnection>(getPool(), transactionRegistry, isAccessToUnderlyingConnectionAllowed()) { + @Override + public void close() throws SQLException { + if (getDelegateInternal() == null) { + return; + } + super.close(); + } + }; + } + }; + pds.setAccessToUnderlyingConnectionAllowed(isAccessToUnderlyingConnectionAllowed()); + return pds; + } + + @Override public int getLoginTimeout() throws SQLException { return 0; } http://git-wip-us.apache.org/repos/asf/tomee/blob/f4a8864b/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..4b32898 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/DbcpNPEXAConnectionTest.java @@ -0,0 +1,114 @@ +/** + * 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.dbcp2.PoolableConnection; +import org.apache.commons.pool2.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<PoolableConnection> 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; + } +} +
