Commons Dbcp
  1. Commons Dbcp
  2. DBCP-369

Exception when using SharedPoolDataSource concurrently

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 1.5.1, 2.0
    • Labels:
      None
    • Environment:

      Java Hotspot Server VM 1.6.0_07, Linux

      Description

      Hi,

      I get a ConcurrentModificationException when using instances of SharedPoolDataSource concurrently. The problem occurs when one thread calls setDataSourceName() while another thread calls close(), either on the same instance of SharedPoolDataSource or on different instances. I'll attach a short example program that leads to an exception for almost every run on my machine.

      The cause of the exception seems to be in InstanceKeyObjectFactory. registerNewInstance() is synchronized, but removeInstance() isn't. This allows concurrent accesses to the non-thread-safe 'instanceMap'.

      Is this a bug? Or are SharedPoolDataSource and InstanceKeyObjectFactory not supposed to be thread-safe?

      Thanks!

      import java.util.ArrayList;
      import org.apache.commons.dbcp.datasources.SharedPoolDataSource;

      public class SharedPoolDataSourceTest {

      public static void main(String[] args)

      { new SharedPoolDataSourceTest().run(); }

      private void run() {
      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() {
      public void run() {
      for (SharedPoolDataSource dataSource : dataSources)

      { dataSource.setDataSourceName("a"); }


      }
      });
      Thread t2 = new Thread(new Runnable() {
      public void run() {
      for (SharedPoolDataSource dataSource : dataSources) {
      try

      { dataSource.close(); }

      catch (Exception e) {}
      }

      }
      });
      t1.start();
      t2.start();
      try

      { t1.join(); t2.join(); }

      catch (InterruptedException ie) {}
      }
      }

      Exception in thread "Thread-0" java.util.ConcurrentModificationException
      at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793)
      at java.util.HashMap$KeyIterator.next(HashMap.java:828)
      at org.apache.commons.dbcp.datasources.InstanceKeyObjectFactory.registerNewInstance(InstanceKeyObjectFactory.java:51)
      at org.apache.commons.dbcp.datasources.InstanceKeyDataSource.setDataSourceName(InstanceKeyDataSource.java:246)
      at potentialBugs.SharedPoolDataSourceTest$1.run(SharedPoolDataSourceTest.java:23)
      at java.lang.Thread.run(Thread.java:619)

      1. DBCP-369.patch
        3 kB
        Thomas Neidhart

        Activity

        Hide
        Mark Thomas added a comment -

        Thanks for the report and the test case. Sorry it has taken so long to address this.

        I've fixed trunk for the 2.x series the 1.5.x branch. It will be included in the next release of each.

        If there are further 1.3.x and 1.4.x releases (currently being discussed on the dev mailing list) my expectation is that they will be generated from the 1.5.x branch so will pick up this fix too.

        Show
        Mark Thomas added a comment - Thanks for the report and the test case. Sorry it has taken so long to address this. I've fixed trunk for the 2.x series the 1.5.x branch. It will be included in the next release of each. If there are further 1.3.x and 1.4.x releases (currently being discussed on the dev mailing list) my expectation is that they will be generated from the 1.5.x branch so will pick up this fix too.
        Hide
        Thomas Neidhart added a comment -

        The access to the InstanceKeyObjectFactory is not fully synchronized. Some methods (registerNewInstance) were already synchronized, while others (removeInstance, closeAll) not.

        This lead to the ConcurrentModificationExceptions when multiple threads try to modify the internal instanceMap.

        The attached patch adds a unit test and fixes the problem by synchronizing all public/protected methods in the factory that modify / access the instanceMap.

        Show
        Thomas Neidhart added a comment - The access to the InstanceKeyObjectFactory is not fully synchronized. Some methods (registerNewInstance) were already synchronized, while others (removeInstance, closeAll) not. This lead to the ConcurrentModificationExceptions when multiple threads try to modify the internal instanceMap. The attached patch adds a unit test and fixes the problem by synchronizing all public/protected methods in the factory that modify / access the instanceMap.

          People

          • Assignee:
            Unassigned
            Reporter:
            Michael Pradel
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development