You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by da...@apache.org on 2008/06/16 22:25:17 UTC
svn commit: r668290 - in /commons/proper/dbcp/trunk/src:
java/org/apache/commons/dbcp/ test/org/apache/commons/dbcp/
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;
}