DO NOT REPLY [Bug 17200] New: - DBCP: org.apache.commons.dbcp.cpdsadapter.PooledConnectionImpl.close() does not always close physical connection

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17200>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17200

DBCP: org.apache.commons.dbcp.cpdsadapter.PooledConnectionImpl.close() does not always close physical connection

           Summary: DBCP:
                    org.apache.commons.dbcp.cpdsadapter.PooledConnectionImpl
                    .close() does not always close physical connection
           Product: Commons
           Version: Nightly Builds
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: Major
          Priority: Other
         Component: Dbcp
        AssignedTo: commons-dev@jakarta.apache.org
        ReportedBy: remke.rutgers@brightalley.nl


The code in file revision 1.2 of 
org.apache.commons.dbcp.cpdsadapter.PooledConnectionImpl for the close() method 
is:

    /**
     * Closes the physical connection and marks this 
     * <code>PooledConnection</code> so that it may not be used 
     * to generate any more logical <code>Connection</code>s.
     *
     * @exception SQLException if an error occurs
     */
    public void close()
        throws SQLException
    {        
        assertOpen();
        isClosed = true;
        if (pstmtPool != null) 
        {
            try
            {
                try
                {
                    pstmtPool.close();
                }
                finally
                {
                    pstmtPool = null;
                    connection.close();
                }
            }
            catch (Exception e)
            {
                if (e instanceof RuntimeException) 
                {
                    throw (RuntimeException)e;
                }
                else 
                {
                    throw new SQLException(e.getMessage());
                }
            }
        }
    }
_____________________________

I am not using prepared statement pooling, so in my case
connection.close();
never gets called.
The connections are marked as closed however, so the number of physical 
connections goes up and up and up...

I modified the close() method in the following way.

    /**
     * Closes the physical connection and marks this 
     * <code>PooledConnection</code> so that it may not be used 
     * to generate any more logical <code>Connection</code>s.
     *
     * @exception SQLException if an error occurs
     */
    public void close()
        throws SQLException
    {        
        assertOpen();
        isClosed = true;
        try
        {
			if (pstmtPool != null) 
			{
                try
                {
                    pstmtPool.close();
                }
                finally
                {
                    pstmtPool = null;
                }
			}
        }
        catch (Exception e)
        {
            if (e instanceof RuntimeException) 
            {
                throw (RuntimeException)e;
            }
            else 
            {
                throw new SQLException(e.getMessage());
            }
        }
		finally
		{
			connection.close();
		}
    }

___________________
I would welcome comments, remarks, improvements on this patch.
And if my changed are judged to be correct, can someone submit this to cvs?

Regards,

Remke Rutgers

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org