You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by rm...@apache.org on 2016/07/05 10:55:52 UTC

tomee git commit: TOMEE-1860 DBCP pool can lead to a NPE for ManagedConnection closing a shared connection

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 <rm...@gmail.com>
Authored: Tue Jul 5 12:55:29 2016 +0200
Committer: Romain manni-Bucau <rm...@gmail.com>
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;
+    }
+}
+