commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d...@apache.org
Subject svn commit: r668290 - in /commons/proper/dbcp/trunk/src: java/org/apache/commons/dbcp/ test/org/apache/commons/dbcp/
Date Mon, 16 Jun 2008 20:25:17 GMT
Author: dain
Date: Mon Jun 16 13:25:17 2008
New Revision: 668290

URL: http://svn.apache.org/viewvc?rev=668290&view=rev
Log:
Fixed DBCP-269 JDBC connection never closes
When closing a PoolableConnection the decision to return or destroy the proxy should be based
on the underlying connection and not the proxy state
The connection proxy should be closable multiple times without throwing an exception

Modified:
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolableConnection.java
    commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSource.java
    commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestDelegatingConnection.java
    commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterConnection.java

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java?rev=668290&r1=668289&r2=668290&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java Mon
Jun 16 13:25:17 2008
@@ -209,15 +209,10 @@
      * any Statements that were not explicitly closed.
      */
     public void close() throws SQLException {
-        // close can be called multiple times, but PoolableConnection improperly
-        // throws an exception when a connection is closed twice, so before calling
-        // close we aren't already closed
-        if (!isClosed()) {
-            try {
-                _conn.close();
-            } finally {
-                _closed = true;
-            }
+        try {
+            _conn.close();
+        } finally {
+            _closed = true;
         }
     }
 
@@ -350,10 +345,7 @@
     { checkOpen(); try { _conn.setTypeMap(map); } catch (SQLException e) { handleException(e);
} }
 
     public boolean isClosed() throws SQLException {
-         if(_closed || _conn.isClosed()) {
-             return true;
-         }
-         return false;
+        return _closed || _conn.isClosed();
     }
 
     protected void checkOpen() throws SQLException {

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolableConnection.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolableConnection.java?rev=668290&r1=668289&r2=668290&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolableConnection.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolableConnection.java Mon
Jun 16 13:25:17 2008
@@ -53,8 +53,7 @@
      * @param config the abandoned configuration settings
      * @deprecated AbandonedConfig is now deprecated.
      */
-    public PoolableConnection(Connection conn, ObjectPool pool,
-                              AbandonedConfig config) {
+    public PoolableConnection(Connection conn, ObjectPool pool, AbandonedConfig config) {
         super(conn, config);
         _pool = pool;
     }
@@ -64,9 +63,14 @@
      * Returns me to my pool.
      */
      public synchronized void close() throws SQLException {
-        boolean isClosed = false;
+        if (_closed) {
+            // already closed
+            return;
+        }
+
+        boolean isUnderlyingConectionClosed;
         try {
-            isClosed = isClosed();
+            isUnderlyingConectionClosed = _conn.isClosed();
         } catch (SQLException e) {
             try {
                 _pool.invalidateObject(this); // XXX should be guarded to happen at most
once
@@ -77,33 +81,38 @@
             } catch (Exception ie) {
                 // DO NOTHING the original exception will be rethrown
             }
-            throw new SQLNestedException("Cannot close connection (isClosed check failed)",
e);
+            throw (SQLException) new SQLException("Cannot close connection (isClosed check
failed)").initCause(e);
         }
-        if (isClosed) {
+
+        if (!isUnderlyingConectionClosed) {
+            // Normal close: underlying connection is still open, so we
+            // simply need to return this proxy to the pool
             try {
-                _pool.invalidateObject(this); // XXX should be guarded to happen at most
once
+                _pool.returnObject(this); // XXX should be guarded to happen at most once
             } catch(IllegalStateException e) {
                 // pool is closed, so close the connection
                 passivate();
                 getInnermostDelegate().close();
-            } catch (Exception ie) {
-                // DO NOTHING, "Already closed" exception thrown below
+            } catch(SQLException e) {
+                throw e;
+            } catch(RuntimeException e) {
+                throw e;
+            } catch(Exception e) {
+                throw (SQLException) new SQLException("Cannot close connection (return to
pool failed)").initCause(e);
             }
-            throw new SQLException("Already closed.");
         } else {
+            // Abnormal close: underlying connection closed unexpectedly, so we
+            // must destroy this proxy
             try {
-                _pool.returnObject(this); // XXX should be guarded to happen at most once
+                _pool.invalidateObject(this); // XXX should be guarded to happen at most
once
             } catch(IllegalStateException e) {
                 // pool is closed, so close the connection
                 passivate();
                 getInnermostDelegate().close();
-            } catch(SQLException e) {
-                throw e;
-            } catch(RuntimeException e) {
-                throw e;
-            } catch(Exception e) {
-                throw new SQLNestedException("Cannot close connection (return to pool failed)",
e);
+            } catch (Exception ie) {
+                // DO NOTHING, "Already closed" exception thrown below
             }
+            throw new SQLException("Already closed.");
         }
     }
 
@@ -113,6 +122,5 @@
     public void reallyClose() throws SQLException {
         super.close();
     }
-
 }
 

Modified: commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSource.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSource.java?rev=668290&r1=668289&r2=668290&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSource.java (original)
+++ commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSource.java Mon
Jun 16 13:25:17 2008
@@ -95,7 +95,7 @@
         ds.close();
 
         // raw idle connection should now be closed
-        assertFalse(rawIdleConnection.isClosed());
+        assertTrue(rawIdleConnection.isClosed());
 
         // active connection should still be open
         assertFalse(activeConnection.isClosed());
@@ -339,7 +339,7 @@
             fail("Expected SQLException");
         }
         catch(SQLException ex) { }
-        
+
         assertEquals(0, ds.getNumActive());
     }
     

Modified: commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestDelegatingConnection.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestDelegatingConnection.java?rev=668290&r1=668289&r2=668290&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestDelegatingConnection.java
(original)
+++ commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestDelegatingConnection.java
Mon Jun 16 13:25:17 2008
@@ -124,7 +124,7 @@
             conn.prepareStatement("");
             fail("Expecting SQLException");
         } catch (SQLException ex) {
-            assertTrue(ex.getMessage().endsWith("invalid PoolingConnection."));
+            assertTrue(ex.getMessage().endsWith("is closed."));
         }  
         
         try {

Modified: commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterConnection.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterConnection.java?rev=668290&r1=668289&r2=668290&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterConnection.java (original)
+++ commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterConnection.java Mon Jun
16 13:25:17 2008
@@ -65,7 +65,7 @@
     }
 
     public void close() throws SQLException {
-        checkOpen();
+        checkFailure();
         _open = false;
     }
 



Mime
View raw message