Commons Dbcp
  1. Commons Dbcp
  2. DBCP-412

dbcp2.PoolableConnection.close raises NullPointerException

    Details

      Description

      I found a critical error while closing a PoolableConnection.
      Here's the code to reproduce the bug (largely copied from the example shown in the Apache DBCP site):

      PoolingDataSourceExample2.java
      public static void main(String[] args) throws Throwable {
          Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver");
          DataSource dataSource = setupDataSource(jdbcUrl);
      
          Connection connection = null;
          PreparedStatement statement = null;
          ResultSet result = null;
      
          try {
              connection = dataSource.getConnection();
              statement = connection.prepareStatement("SELECT 1");
              result = statement.executeQuery();
          } finally {
              result.close();
              statement.close();
              connection.close();
          }
      }
      
      public static DataSource setupDataSource(String connectURI) {
          ConnectionFactory connectionFactory = new DriverManagerConnectionFactory(connectURI, null);
          PoolableConnectionFactory poolableConnectionFactory = new PoolableConnectionFactory(connectionFactory, null);
          ObjectPool<PoolableConnection> connectionPool = new GenericObjectPool<>(poolableConnectionFactory);
          PoolingDataSource<PoolableConnection> dataSource = new PoolingDataSource<>(connectionPool);
      
          return dataSource;
      }
      

      When the code tries to close the connection (in the final block), an exception is raised:

      PoolingDataSourceExample2.java
      Exception in thread "main" java.lang.NullPointerException
      	at org.apache.commons.dbcp2.PoolableConnection.close(PoolableConnection.java:151)
      	at org.apache.commons.dbcp2.DelegatingConnection.closeInternal(DelegatingConnection.java:235)
      	at org.apache.commons.dbcp2.DelegatingConnection.close(DelegatingConnection.java:218)
      	at org.apache.commons.dbcp2.PoolingDataSource$PoolGuardConnectionWrapper.close(PoolingDataSource.java:212)
      	at dbcp.PoolingDataSourceExample2.closeQuietly(PoolingDataSourceExample2.java:64)
      	at dbcp.PoolingDataSourceExample2.main(PoolingDataSourceExample2.java:43)

      As I can see, the problem is the "_pool" variable inside PoolableConnection but I could not find any solution.

      1. DBCP-412.patch
        3 kB
        Phil Steitz

        Activity

        Hide
        Phil Steitz added a comment -

        Fixed examples in r1659726.

        Show
        Phil Steitz added a comment - Fixed examples in r1659726.
        Hide
        Phil Steitz added a comment -

        Thanks for pointing out the inconsistency in current code in the examples, Sebastien. The check added in r1592119 makes it unnecessary to add the setPool (once this code is released). Reopening as we should make the examples consistent. I think we should add the setPool calls for now as current released code requires them. Will get to this ASAP unless someone beats me to it.

        Show
        Phil Steitz added a comment - Thanks for pointing out the inconsistency in current code in the examples, Sebastien. The check added in r1592119 makes it unnecessary to add the setPool (once this code is released). Reopening as we should make the examples consistent. I think we should add the setPool calls for now as current released code requires them. Will get to this ASAP unless someone beats me to it.
        Hide
        Sebastian Hinckel added a comment -

        The same problem still exists in the examples for the pooling driver. Adding

        poolableConnectionFactory.setPool(connectionPool)

        solves the problem here, too.

        Show
        Sebastian Hinckel added a comment - The same problem still exists in the examples for the pooling driver. Adding poolableConnectionFactory.setPool(connectionPool) solves the problem here, too.
        Hide
        Phil Steitz added a comment -

        PDS constructor check added in r1592119

        Show
        Phil Steitz added a comment - PDS constructor check added in r1592119
        Hide
        Phil Steitz added a comment -

        Thinking about this some more, I think it would be a good idea to have the PoolingDataSource constructor check and if necessary fix the factory-pool linkage. Attached patch does this. Comments / better ideas welcome.

        Show
        Phil Steitz added a comment - Thinking about this some more, I think it would be a good idea to have the PoolingDataSource constructor check and if necessary fix the factory-pool linkage. Attached patch does this. Comments / better ideas welcome.
        Hide
        Phil Steitz added a comment -

        Sample code in PoolingDataSourceExample fixed in r1584958.

        Still to do:

        • Update other examples (incl header comments)
        • Add unit tests to verify examples work - for this example, either TesterDriver has to modified to handle uid/pwd in URL or example has to set props
        Show
        Phil Steitz added a comment - Sample code in PoolingDataSourceExample fixed in r1584958. Still to do: Update other examples (incl header comments) Add unit tests to verify examples work - for this example, either TesterDriver has to modified to handle uid/pwd in URL or example has to set props
        Hide
        Davide Caroselli added a comment -

        Ok I've made a test as you suggested and you're right! Although the connection wrapper is different everytime, the open connections db-side are exactly equals to the value set by setMaxTotal. Great, thanks!

        Show
        Davide Caroselli added a comment - Ok I've made a test as you suggested and you're right! Although the connection wrapper is different everytime, the open connections db-side are exactly equals to the value set by setMaxTotal. Great, thanks!
        Hide
        Phil Steitz added a comment -

        The difference in behavior is likely due to the changes in DBCP-358. The hashcodes you are examining are those of the DelegatingConnection instances returned by PoolingDataSource. As of 2.0, DelegatingConnection instances wrapping the same underlying physical connection are not equal. If you have a way to see the physical connections in the database, you should be able to verify that there is just one being used by the code (else there is a bug here).

        Show
        Phil Steitz added a comment - The difference in behavior is likely due to the changes in DBCP-358 . The hashcodes you are examining are those of the DelegatingConnection instances returned by PoolingDataSource. As of 2.0, DelegatingConnection instances wrapping the same underlying physical connection are not equal. If you have a way to see the physical connections in the database, you should be able to verify that there is just one being used by the code (else there is a bug here).
        Hide
        Davide Caroselli added a comment - - edited

        Ok, I've modified the code but I found another issue (maybe). This is the example:

        PoolingDataSourceExample2.java
        public static void main(String[] args) throws Throwable {
            Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver");
            DataSource dataSource = setupDataSource(jdbcUrl);
        
            Connection connection = null;
            PreparedStatement statement = null;
            ResultSet result = null;
        
            // = EDITED ==============
            for (int i = 0; i < 100; i++)
                try {
                    connection = dataSource.getConnection();
                    System.out.println(connection.hashCode());
                    statement = connection.prepareStatement("SELECT 1");
                    result = statement.executeQuery();
                } finally {
                    result.close();
                    statement.close();
                    connection.close();
                }
        }
        
        public static DataSource setupDataSource(String connectURI) {
            ConnectionFactory connectionFactory = new DriverManagerConnectionFactory(connectURI, null);
            PoolableConnectionFactory poolableConnectionFactory = new PoolableConnectionFactory(connectionFactory, null);
            ObjectPool<PoolableConnection> connectionPool = new GenericObjectPool<>(poolableConnectionFactory);
        
            // = EDITED ==============
            connectionPool.setMaxTotal(1);
            poolableConnectionFactory.setPool(connectionPool);
        
            PoolingDataSource<PoolableConnection> dataSource = new PoolingDataSource<>(connectionPool);
            return dataSource;
        }
        

        Now the code does not throw exceptions anymore, but it returns always a new connection.
        I request 100 times, but every time the connection hashcode is different; I've made a similar example but with DBCP v1.4 over Common Pool 1.6 and with these jars the hashcode is always the same.

        Is this normal or does it really create new connection anytime?

        Show
        Davide Caroselli added a comment - - edited Ok, I've modified the code but I found another issue (maybe). This is the example: PoolingDataSourceExample2.java public static void main( String [] args) throws Throwable { Class .forName( "com.microsoft.sqlserver.jdbc.SQLServerDriver" ); DataSource dataSource = setupDataSource(jdbcUrl); Connection connection = null ; PreparedStatement statement = null ; ResultSet result = null ; // = EDITED ============== for ( int i = 0; i < 100; i++) try { connection = dataSource.getConnection(); System .out.println(connection.hashCode()); statement = connection.prepareStatement( "SELECT 1" ); result = statement.executeQuery(); } finally { result.close(); statement.close(); connection.close(); } } public static DataSource setupDataSource( String connectURI) { ConnectionFactory connectionFactory = new DriverManagerConnectionFactory(connectURI, null ); PoolableConnectionFactory poolableConnectionFactory = new PoolableConnectionFactory(connectionFactory, null ); ObjectPool<PoolableConnection> connectionPool = new GenericObjectPool<>(poolableConnectionFactory); // = EDITED ============== connectionPool.setMaxTotal(1); poolableConnectionFactory.setPool(connectionPool); PoolingDataSource<PoolableConnection> dataSource = new PoolingDataSource<>(connectionPool); return dataSource; } Now the code does not throw exceptions anymore, but it returns always a new connection. I request 100 times, but every time the connection hashcode is different; I've made a similar example but with DBCP v1.4 over Common Pool 1.6 and with these jars the hashcode is always the same. Is this normal or does it really create new connection anytime?
        Hide
        Phil Steitz added a comment - - edited

        It might also a good idea to have the PoolingDataSource constructor check that its pool's factory refers back to itself.

        Show
        Phil Steitz added a comment - - edited It might also a good idea to have the PoolingDataSource constructor check that its pool's factory refers back to itself.
        Hide
        Phil Steitz added a comment -

        The problem is in the example code. In version 2, the PoolableConnectionFactory constructor no longer takes the parent pool as a parameter. To correctly set up the pool manually, we need to set the pool property of the PoolableConnectionFactory using its setPool method. So above we need to add

         poolableConnectionFactory.setPool(connectionPool) 

        We should fix this in the example and add unit tests confirming that the examples work. Patches welcome.

        Show
        Phil Steitz added a comment - The problem is in the example code. In version 2, the PoolableConnectionFactory constructor no longer takes the parent pool as a parameter. To correctly set up the pool manually, we need to set the pool property of the PoolableConnectionFactory using its setPool method. So above we need to add poolableConnectionFactory.setPool(connectionPool) We should fix this in the example and add unit tests confirming that the examples work. Patches welcome.

          People

          • Assignee:
            Unassigned
            Reporter:
            Davide Caroselli
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development