You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@commons.apache.org by Nathan Smith <na...@labpro2000.com> on 2005/10/27 23:35:43 UTC
[DBCP] HIGH PRIORITY. Code changes to make sure connections are closed.
To the DBCP development team,
A while back we had a problem with DBCP not closing connections fully.
It went throught the motions of closing the connection, adjusting the
active, and inuse counters correctly but the connection never actually
was closed. It was however no longer referenced by DBCP.
I'll try to explain why this problem occured in the first place. A few
years ago we read some documentation that said it was always a good idea
to explicitly close any ResultSet's or Statements that you use to
release resources as soon as you have finished with then, even though
the javadoc states the ResultSet is closed when the Statement is closed.
We have done this throughout the application. Our JDBC driver likes to
throw SQLExceptions if a ResultSet or Statement have already been closed
when the Connection is closed, as it is supposed to clean up after
itself as per the javadoc.
This causes problems in the DelegatingConnection class when the
connection is really closed. It calls passivate(), which closes the
Statement's and ResultSet's that are associated with the Connection. If
the Statement is already closed it could throw an NPE or most likely an
SQLException because the ResultSet is already closed. This misses out
the call _conn.close(). The single most crucial call that tells the
database to close this connection. After a while the database can no
longer provide connections as it has reached a specified limit but the
connection pool numbers look as though everything is fine.
I have included the changes I have made to the code. I don't know how to
create a patch to merge into your development tree, sorry.
Below is the modified code:
DelegatingConnection.java
public void close() throws SQLException
{
* log.debug("Closes the underlying connection, and closes "
+ "any Statements that were not explicitly closed.");
try {
passivate();
} catch(SQLException sqle) {
log.warn("Problem passivating connection. " +
sqle.getMessage(), sqle);
throw sqle;
}
finally {
log.debug("Closing the underlying connection ("+ _conn + ')');
if(_conn != null) {
_conn.close();
}
}*
}
protected void passivate() throws SQLException {
log.debug("[DelegatingConnection.passivate] Connection (" +
_conn + ')');
try {
// The JDBC spec requires that a Connection close any open
// Statement's when it is closed.
List statements = getTrace();
log.debug("statements: " + statements);
if( statements != null) {
Statement[] set = new Statement[statements.size()];
statements.toArray(set);
for (int i = 0; i < set.length; i++) {
* if(set[i] != null) {
set[i].close();
}*
}
clearTrace();
}
setLastUsed(0);
if(_conn instanceof DelegatingConnection) {
((DelegatingConnection)_conn).passivate();
}
}
finally {
_closed = true;
}
}
DelegatingStatement.java
public void close() throws SQLException {
try {
try {
if (_conn != null) {
_conn.removeTrace(this);
_conn = null;
}
// The JDBC spec requires that a statment close any open
// ResultSet's when it is closed.
// FIXME The PreparedStatement we're wrapping should
handle this for us.
// See bug 17301 for what could happen when ResultSets
are closed twice.
List resultSets = getTrace();
if( resultSets != null) {
ResultSet[] set = (ResultSet[])
resultSets.toArray(new ResultSet[resultSets.size()]);
for (int i = 0; i < set.length; i++) {
* if(set[i] != null) {
set[i].close();
}*
}
clearTrace();
}
* if(_stmt != null) {
_stmt.close();
}*
}
catch (SQLException e) {
handleException(e);
}
}
finally {
_closed = true;
}
}
DelegatingResultSet.java
public void close() throws SQLException {
try {
if(_stmt != null) {
((AbandonedTrace)_stmt).removeTrace(this);
_stmt = null;
}
*if(_res != null) {
_res.close();
}*
}
catch (SQLException e) {
handleException(e);
}
PoolableConnectionFactory.java
public void validateConnection(Connection conn) throws SQLException {
String query = _validationQuery;
if(conn.isClosed()) {
throw new SQLException("validateConnection: connection closed");
}
if(null != query) {
Statement stmt = null;
ResultSet rset = null;
try {
stmt = conn.createStatement();
rset = stmt.executeQuery(query);
if(!rset.next()) {
throw new SQLException("validationQuery didn't
return a row");
}
} finally {
try {
*if(rset != null) {
rset.close();
}*
} catch(Exception t) {
// ignored
}
try {
*if(stmt != null) {
stmt.close();
}*
} catch(Exception t) {
// ignored
}
}
}
}
PoolingConnection.java
public synchronized void close() throws SQLException {
log.debug("[PoolingConnection.close] ");
*try {*
if(null != _pstmtPool) {
KeyedObjectPool oldpool = _pstmtPool;
_pstmtPool = null;
try {
oldpool.close();
} catch(RuntimeException e) {
throw e;
} catch(SQLException e) {
throw e;
} catch(Exception e) {
throw new SQLNestedException("Cannot close
connection", e);
}
}
* } finally {
Connection innermostDelegate = getInnermostDelegate();
log.debug("Closing my underlying connection (" +
innermostDelegate + ')');
if(innermostDelegate != null) {
innermostDelegate.close();
}
}*
}
Cheers.
--
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [DBCP] HIGH PRIORITY. Code changes to make sure connections are
closed.
Posted by Nathan Smith <na...@labpro2000.com>.
Hi Dave,
Thanks for the heads up about c3po. Im gonna download it and integrate
into our app to give it a test drive and see how it goes.
I also downloaded the datasource component of Apache Excalibur. I might
see how that goes as well (when I find the time).
Cheers.
David Durham wrote:
>On 10/27/05, Nathan Smith <na...@labpro2000.com> wrote:
>
>
>>Hi Dave,
>>
>>I have been wondering whether it's worth while or not carrying on using
>>DBCP. There are many outstanding bugs and hardly any active development
>>on it. One release a year just doesn't cut it for me.
>>
>>I would suggest the Tomcat team also use something other than DBCP,
>>cause it makes them look bad when the database side of things doesn't
>>work. People start blaming the Tomcat when it isn't there problem at all.
>>
>>
>
>Well, I guess they are probably not concerned with looking bad ... I
>know I wouldn't be -- Tomcat is a nice tool. I just mention c3p0
>because it and another one (can't remember the name) are getting more
>recommendations these days than DBCP. And like you said, DBCP doesn't
>seem to be getting a lot of developer attention. I'm not sure that
>Tomcat would ship with anything under a non-Apache license agreement,
>though. I could be wrong.
>
>
>-Dave
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-user-help@jakarta.apache.org
>
>
>
>
--
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [DBCP] HIGH PRIORITY. Code changes to make sure connections are closed.
Posted by David Durham <da...@gmail.com>.
On 10/27/05, Nathan Smith <na...@labpro2000.com> wrote:
> Hi Dave,
>
> I have been wondering whether it's worth while or not carrying on using
> DBCP. There are many outstanding bugs and hardly any active development
> on it. One release a year just doesn't cut it for me.
>
> I would suggest the Tomcat team also use something other than DBCP,
> cause it makes them look bad when the database side of things doesn't
> work. People start blaming the Tomcat when it isn't there problem at all.
Well, I guess they are probably not concerned with looking bad ... I
know I wouldn't be -- Tomcat is a nice tool. I just mention c3p0
because it and another one (can't remember the name) are getting more
recommendations these days than DBCP. And like you said, DBCP doesn't
seem to be getting a lot of developer attention. I'm not sure that
Tomcat would ship with anything under a non-Apache license agreement,
though. I could be wrong.
-Dave
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [DBCP] HIGH PRIORITY. Code changes to make sure connections are
closed.
Posted by Nathan Smith <na...@labpro2000.com>.
Hi Dave,
I have been wondering whether it's worth while or not carrying on using
DBCP. There are many outstanding bugs and hardly any active development
on it. One release a year just doesn't cut it for me.
I would suggest the Tomcat team also use something other than DBCP,
cause it makes them look bad when the database side of things doesn't
work. People start blaming the Tomcat when it isn't there problem at all.
Cheers.
David Durham wrote:
>On 10/27/05, Nathan Smith <na...@labpro2000.com> wrote:
>
>
>>To the DBCP development team,
>>
>>A while back we had a problem with DBCP not closing connections fully.
>>It went throught the motions of closing the connection, adjusting the
>>active, and inuse counters correctly but the connection never actually
>>was closed. It was however no longer referenced by DBCP.
>>
>>
>
>
>You might try using c3pO, which can be found over at sourceforge.
>
>
>-Dave
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-user-help@jakarta.apache.org
>
>
>
>
--
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org
Re: [DBCP] HIGH PRIORITY. Code changes to make sure connections are closed.
Posted by David Durham <da...@gmail.com>.
On 10/27/05, Nathan Smith <na...@labpro2000.com> wrote:
> To the DBCP development team,
>
> A while back we had a problem with DBCP not closing connections fully.
> It went throught the motions of closing the connection, adjusting the
> active, and inuse counters correctly but the connection never actually
> was closed. It was however no longer referenced by DBCP.
You might try using c3pO, which can be found over at sourceforge.
-Dave
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org