commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1562992 - in /commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src: changes/changes.xml main/java/org/apache/commons/dbcp/datasources/InstanceKeyObjectFactory.java test/java/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
Date Thu, 30 Jan 2014 21:54:56 GMT
Author: markt
Date: Thu Jan 30 21:54:56 2014
New Revision: 1562992

URL: http://svn.apache.org/r1562992
Log:
Fix DBCP-369
Fix threading issue when using multiple instances of the SharedPoolDataSource concurrently.
I can't reproduce this but the issue can be seen from looking at the code.

Modified:
    commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/changes/changes.xml
    commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/main/java/org/apache/commons/dbcp/datasources/InstanceKeyObjectFactory.java
    commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/test/java/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java

Modified: commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/changes/changes.xml?rev=1562992&r1=1562991&r2=1562992&view=diff
==============================================================================
--- commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/changes/changes.xml (original)
+++ commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/changes/changes.xml Thu Jan 30 21:54:56
2014
@@ -74,6 +74,10 @@ The <action> type attribute can be add,u
         Allow accessToUnderlyingConnectionAllowed to be configured when
         configuration takes place via JNDI in a JavaEE container.
       </action>
+      <action dev="markt" issue="DBCP-369" type="fix" due-to="Michael Pradel">
+        Fix threading issue when using multiple instances of the
+        SharedPoolDataSource concurrently.
+      </action>
     </release>
     <release version="1.4.1" date="TBD" description="TBD">
       <action dev="psteitz" issue="DBCP-367" type="fix" due-to="Ken Tatsushita">

Modified: commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/main/java/org/apache/commons/dbcp/datasources/InstanceKeyObjectFactory.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/main/java/org/apache/commons/dbcp/datasources/InstanceKeyObjectFactory.java?rev=1562992&r1=1562991&r2=1562992&view=diff
==============================================================================
--- commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/main/java/org/apache/commons/dbcp/datasources/InstanceKeyObjectFactory.java
(original)
+++ commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/main/java/org/apache/commons/dbcp/datasources/InstanceKeyObjectFactory.java
Thu Jan 30 21:54:56 2014
@@ -14,7 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.commons.dbcp.datasources;
 
 import java.io.ByteArrayInputStream;
@@ -23,9 +22,9 @@ import java.io.ObjectInputStream;
 
 import java.util.Hashtable;
 import java.util.Map;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
 
 import javax.naming.Context;
 import javax.naming.Name;
@@ -39,10 +38,9 @@ import javax.naming.spi.ObjectFactory;
  *
  * @version $Revision$ $Date$
  */
-abstract class InstanceKeyObjectFactory
-    implements ObjectFactory
-{
-    private static final Map instanceMap = new HashMap();
+abstract class InstanceKeyObjectFactory implements ObjectFactory {
+
+    private static final Map instanceMap = new ConcurrentHashMap();
 
     synchronized static String registerNewInstance(InstanceKeyDataSource ds) {
         int max = 0;

Modified: commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/test/java/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/test/java/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java?rev=1562992&r1=1562991&r2=1562992&view=diff
==============================================================================
--- commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/test/java/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
(original)
+++ commons/proper/dbcp/branches/DBCP_1_5_x_BRANCH/src/test/java/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
Thu Jan 30 21:54:56 2014
@@ -5,9 +5,9 @@
  * 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.
@@ -21,6 +21,7 @@ import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.ArrayList;
 
 import javax.sql.DataSource;
 
@@ -79,14 +80,14 @@ public class TestSharedPoolDataSource ex
 
     /**
      * Switching 'u1 -> 'u2' and 'p1' -> 'p2' will
-     * exhibit the bug detailed in 
+     * exhibit the bug detailed in
      * http://issues.apache.org/bugzilla/show_bug.cgi?id=18905
-     * 
+     *
      * Starting with a successful connection, then incorrect password,
      * then correct password for same user illustrates
      * JIRA: DBCP-245
      */
-    public void testIncorrectPassword() throws Exception 
+    public void testIncorrectPassword() throws Exception
     {
         ds.getConnection("u2", "p2").close();
         try {
@@ -97,26 +98,26 @@ public class TestSharedPoolDataSource ex
             // should fail
 
         }
-        
+
         // Use good password
         ds.getConnection("u1", "p1").close();
-        try 
+        try
         {
             ds.getConnection("u1", "x");
             fail("Able to retrieve connection with incorrect password");
         }
         catch (SQLException e)
         {
-            if (!e.getMessage().startsWith("Given password did not match")) 
+            if (!e.getMessage().startsWith("Given password did not match"))
             {
                 throw e;
             }
             // else the exception was expected
         }
-        
+
         // Make sure we can still use our good password.
         ds.getConnection("u1", "p1").close();
-        
+
         // Try related users and passwords
         ds.getConnection("foo", "bar").close();
         try {
@@ -132,7 +133,7 @@ public class TestSharedPoolDataSource ex
     }
 
 
-    public void testSimple() throws Exception 
+    public void testSimple() throws Exception
     {
         Connection conn = ds.getConnection();
         assertNotNull(conn);
@@ -146,7 +147,7 @@ public class TestSharedPoolDataSource ex
         conn.close();
     }
 
-    public void testSimpleWithUsername() throws Exception 
+    public void testSimpleWithUsername() throws Exception
     {
         Connection conn = ds.getConnection("u1", "p1");
         assertNotNull(conn);
@@ -160,12 +161,12 @@ public class TestSharedPoolDataSource ex
         conn.close();
     }
 
-    public void testClosingWithUserName() 
-        throws Exception 
+    public void testClosingWithUserName()
+        throws Exception
     {
         Connection[] c = new Connection[getMaxActive()];
         // open the maximum connections
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i] = ds.getConnection("u1", "p1");
         }
@@ -176,29 +177,29 @@ public class TestSharedPoolDataSource ex
         // get a new connection
         c[0] = ds.getConnection("u1", "p1");
 
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i].close();
         }
 
         // open the maximum connections
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i] = ds.getConnection("u1", "p1");
         }
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i].close();
         }
     }
 
-    public void testSimple2() 
-        throws Exception 
+    public void testSimple2()
+        throws Exception
     {
         Connection conn = ds.getConnection();
         assertNotNull(conn);
 
-        PreparedStatement stmt = 
+        PreparedStatement stmt =
             conn.prepareStatement("select * from dual");
         assertNotNull(stmt);
         ResultSet rset = stmt.executeQuery();
@@ -206,7 +207,7 @@ public class TestSharedPoolDataSource ex
         assertTrue(rset.next());
         rset.close();
         stmt.close();
-        
+
         stmt = conn.prepareStatement("select * from dual");
         assertNotNull(stmt);
         rset = stmt.executeQuery();
@@ -214,14 +215,14 @@ public class TestSharedPoolDataSource ex
         assertTrue(rset.next());
         rset.close();
         stmt.close();
-        
+
         conn.close();
-        try 
+        try
         {
             conn.createStatement();
             fail("Can't use closed connections");
-        } 
-        catch(SQLException e) 
+        }
+        catch(SQLException e)
         {
             // expected
         }
@@ -244,38 +245,38 @@ public class TestSharedPoolDataSource ex
         assertTrue(rset.next());
         rset.close();
         stmt.close();
-        
+
         conn.close();
         conn = null;
     }
 
-    public void testOpening() 
-        throws Exception 
+    public void testOpening()
+        throws Exception
     {
         Connection[] c = new Connection[getMaxActive()];
         // test that opening new connections is not closing previous
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i] = ds.getConnection();
             assertTrue(c[i] != null);
-            for (int j=0; j<=i; j++) 
+            for (int j=0; j<=i; j++)
             {
                 assertTrue(!c[j].isClosed());
             }
         }
 
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i].close();
         }
     }
 
-    public void testClosing() 
-        throws Exception 
+    public void testClosing()
+        throws Exception
     {
         Connection[] c = new Connection[getMaxActive()];
         // open the maximum connections
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i] = ds.getConnection();
         }
@@ -283,36 +284,36 @@ public class TestSharedPoolDataSource ex
         // close one of the connections
         c[0].close();
         assertTrue(c[0].isClosed());
-        
+
         // get a new connection
         c[0] = ds.getConnection();
 
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i].close();
         }
     }
-    
+
     /**
      * Test pool close.  Illustrates BZ 37359.
-     * 
+     *
      * @throws Exception
      */
     public void testClosePool() throws Exception {
       ((SharedPoolDataSource)ds).close();
       SharedPoolDataSource tds = new SharedPoolDataSource();
-      // NPE before BZ 37359 fix 
+      // NPE before BZ 37359 fix
       tds.close();
     }
 
-    public void testMaxActive() 
-        throws Exception 
+    public void testMaxActive()
+        throws Exception
     {
         Connection[] c = new Connection[getMaxActive()];
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i] = ds.getConnection();
-            assertTrue(c[i] != null);            
+            assertTrue(c[i] != null);
         }
 
         try
@@ -326,49 +327,49 @@ public class TestSharedPoolDataSource ex
             // throw an exception
         }
 
-        for (int i=0; i<c.length; i++) 
+        for (int i=0; i<c.length; i++)
         {
             c[i].close();
         }
     }
-    
+
     public void testMaxWait() throws Exception {
         final int maxWait = 1000;
         final int theadCount = 20;
-        
+
         ((SharedPoolDataSource)ds).setMaxWait(maxWait);
         // Obtain all the connections from the pool
         Connection[] c = new Connection[getMaxActive()];
         for (int i=0; i<c.length; i++) {
             c[i] = ds.getConnection("foo","bar");
-            assertTrue(c[i] != null);            
+            assertTrue(c[i] != null);
         }
 
         long start = System.currentTimeMillis();
-        
+
         // Run a thread test with minimal hold time
         // All threads should end after maxWait - DBCP-291
         final PoolTest[] pts = new PoolTest[theadCount];
         ThreadGroup threadGroup = new ThreadGroup("testMaxWait");
 
-        // Should take ~maxWait for threads to stop 
+        // Should take ~maxWait for threads to stop
         for (int i = 0; i < pts.length; i++) {
             (pts[i] = new PoolTest(threadGroup, 1, true)).start();
         }
-        
+
         // Wait for all the threads to complete
         for (int i = 0; i < pts.length; i++) {
             final PoolTest poolTest = pts[i];
             poolTest.getThread().join();
         }
 
-        
+
         long end = System.currentTimeMillis();
-        
+
         System.out.println("testMaxWait took " + (end-start) + " ms. Maxwait: "+maxWait);
-        
+
         // Threads should time out in parallel - allow double that to be safe
-        assertTrue((end-start) < (2 * maxWait)); 
+        assertTrue((end-start) < (2 * maxWait));
 
         // Put all the connections back in the pool
         for (int i=0; i<c.length; i++) {
@@ -393,25 +394,25 @@ public class TestSharedPoolDataSource ex
     public void testTransactionIsolationBehavior() throws Exception {
         Connection conn = getConnection();
         assertNotNull(conn);
-        assertEquals(Connection.TRANSACTION_READ_COMMITTED, 
+        assertEquals(Connection.TRANSACTION_READ_COMMITTED,
                      conn.getTransactionIsolation());
         conn.setTransactionIsolation(Connection.TRANSACTION_READ_UNCOMMITTED);
         conn.close();
-        
+
         Connection conn2 = getConnection();
-        assertEquals(Connection.TRANSACTION_READ_COMMITTED, 
+        assertEquals(Connection.TRANSACTION_READ_COMMITTED,
                      conn2.getTransactionIsolation());
-        
+
         Connection conn3 = getConnection();
-        assertEquals(Connection.TRANSACTION_READ_COMMITTED, 
+        assertEquals(Connection.TRANSACTION_READ_COMMITTED,
                      conn3.getTransactionIsolation());
         conn2.close();
         conn3.close();
-    }     
+    }
 
-    // Bugzilla Bug 24136 ClassCastException in DriverAdapterCPDS 
+    // Bugzilla Bug 24136 ClassCastException in DriverAdapterCPDS
     // when setPoolPreparedStatements(true)
-    public void testPoolPrepareStatement() throws Exception 
+    public void testPoolPrepareStatement() throws Exception
     {
         pcds.setPoolPreparedStatements(true);
 
@@ -567,25 +568,66 @@ public class TestSharedPoolDataSource ex
         try {
             Connection con4 = ds.getConnection("foo", "bay"); // new password
             // Idle instances with old password should have been cleared
-            assertEquals("Should be no idle connections in the pool", 
+            assertEquals("Should be no idle connections in the pool",
                    0, ((SharedPoolDataSource) ds).getNumIdle());
             con4.close();
             // Should be one idle instance with new pwd
-            assertEquals("Should be one idle connection in the pool", 
+            assertEquals("Should be one idle connection in the pool",
                     1, ((SharedPoolDataSource) ds).getNumIdle());
             try {
                 ds.getConnection("foo", "bar"); // old password
-                fail("Should have generated SQLException"); 
+                fail("Should have generated SQLException");
             } catch (SQLException expected) {
             }
             Connection con5 = ds.getConnection("foo", "bay"); // take the idle one
             con3.close(); // Return a connection with the old password
             ds.getConnection("foo", "bay").close();  // will try bad returned connection
and destroy it
-            assertEquals("Should be one idle connection in the pool", 
+            assertEquals("Should be one idle connection in the pool",
                     1, ((SharedPoolDataSource) ds).getNumIdle());
             con5.close();
         } finally {
             TesterDriver.addUser("foo","bar");
         }
     }
+
+    public void testDbcp369() {
+        final ArrayList<SharedPoolDataSource> dataSources =
+                new ArrayList<SharedPoolDataSource>();
+        for (int j = 0; j < 10000; j++) {
+            SharedPoolDataSource dataSource = new SharedPoolDataSource();
+            dataSources.add(dataSource);
+        }
+
+        Thread t1 = new Thread(new Runnable() {
+            @Override
+            public void run() {
+                for (SharedPoolDataSource dataSource : dataSources) {
+                    dataSource.setDataSourceName("a");
+                }
+            }
+        });
+
+        Thread t2 = new Thread(new Runnable() {
+            @Override
+            public void run() {
+                for (SharedPoolDataSource dataSource : dataSources) {
+                    try {
+                        dataSource.close();
+                    } catch (Exception e) {
+                        // Ignore
+                    }
+                }
+            }
+        });
+
+        t1.start();
+        t2.start();
+
+        try {
+            t1.join();
+            t2.join();
+        } catch (InterruptedException ie) {
+            // Ignore
+        }
+    }
 }



Mime
View raw message