commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: 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 23:12:57 GMT
On 30 January 2014 21:54,  <markt@apache.org> wrote:
> 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();

Does not work - HashMap allows null as a key, but ConcurrentHashMap does not.

And the key can be null - not sure whether that is a feaure of the
unit tests or if it is allowed for regular use as well.

>      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
> +        }
> +    }
>  }
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message