[jira] Issue Comment Edited: (DBCP-329) SQLException: Already closed.

["Hontvari Jozsef (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/DBCP-329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12850960#action_12850960 ] 

Hontvari Jozsef edited comment on DBCP-329 at 3/29/10 4:17 PM:
---------------------------------------------------------------

That is true. But I still feel the message is not right. The original exception which was hidden by the "Already closed" exception is relatively clear (assuming that you know the less then ideal MySQL behaviour regarding closing unused connections.) I guess what happens here is that when in a previous operation an exception was thrown, DBCP declares that the connection is closed. 
There is two cases, depending on how  DBCP works:
1) If it does try to connect the database on the close() operation, then it should throw the real exception. Like the hidden exception: jdbc4.CommunicationsException with message "Communications link failure". This is as informative as it could be (it is the responsibility of MySQL that it cannot be more informative).
2) If I am right, it does not connect the database on close(). It already knows that it was itself who declared that the connection was broken, and it already thrown an exception in a previous operation. We could very well say that DBCP was the one who "closed" the broken connection. In this case it should not throw another exception. I think that is in line with the intent of the Java specification, and DBCP has all necessary informations which allows this behavior.

If we look at from another viewpoint I think this usage pattern should work (and it worked before):
void fn() throws SQLException
{
  Connection connection = ....
  try {
    connection.doSomething();
  } finally {
    connection.close();
  }
}
Currently this throws the misleading "Already closed." exception instead of the more informative jdbc4.CommunicationsException.

      was (Author: hontvari):
    That is true. But I still feel the message is not right. The original exception which was hidden by the "Already closed" exception is relatively clear (assuming that you know the less then ideal MySQL behaviour regarding closing unused connections.) I guess what happens here is that when in a previous operation an exception was thrown, DBCP declares that the connection is closed. 
There is two cases, depending on how  DBCP works:
1) If it does try to connect the database on the close() operation, then it should throw the real exception. Like the hidden exception: jdbc4.CommunicationsException with message "Communications link failure". This is as informative as it could be (it is the responsibility of MySQL that it cannot be more informative).
2) If I am right, it does not connect the database on close(). It already knows that it was itself who declared that the connection was broken, and it already thrown an exception in a previous operation. We could very well say that DBCP was the one who "closed" the broken connection. In this case it should not throw another exception. I think that is in line with the intent of the Java specification, and DBCP has all necessasary informations which allows this behaviour.

If we look at from another viewpoint I think this usage pattern should work (and it worked before):
void fn() throws SQLException
{
  Connection connection = ....
  try {
    connection.doSomething();
  } finally {
    connection.close();
  }
}
Currently this throws the misleading "Already closed." exception instead of the more informative jdbc4.CommunicationsException.
  
> SQLException: Already closed.
> -----------------------------
>
>                 Key: DBCP-329
>                 URL: https://issues.apache.org/jira/browse/DBCP-329
>             Project: Commons Dbcp
>          Issue Type: Bug
>    Affects Versions: 1.4
>         Environment: MySQL
>            Reporter: Hontvari Jozsef
>
> After upgrading to 1.4 I see such exceptions logged:
> java.sql.SQLException: Already closed.
> 	at org.apache.commons.dbcp.PoolableConnection.close(PoolableConnection.java:114)
> 	at org.apache.commons.dbcp.PoolingDataSource$PoolGuardConnectionWrapper.close(PoolingDataSource.java:191)
> 	...
> This should never happen. According to the Connection.close() javadoc: 
> "Calling the method close on a Connection object that is already closed is a no-op."
>  
> Moreover, I am pretty sure that our code does not close the connection twice. But because the close() is called in a finally block, it is possible that this exception hides another exception. Unfortunately I cannot reproduce it, even though it occurs regularly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.