Author: markt Date: Tue Jun 7 19:28:42 2011 New Revision: 1133134 URL: http://svn.apache.org/viewvc?rev=1133134&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51264 Improve fix to return connections to the pool when not in use. Patch provided by Felix Schumacher.
Modified: tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java tomcat/trunk/webapps/docs/changelog.xml tomcat/trunk/webapps/docs/config/manager.xml Modified: tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java?rev=1133134&r1=1133133&r2=1133134&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java (original) +++ tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java Tue Jun 7 19:28:42 2011 @@ -711,16 +711,7 @@ public class JDBCStore extends StoreBase } try { - if (preparedRemoveSql == null) { - String removeSql = "DELETE FROM " + sessionTable - + " WHERE " + sessionIdCol + " = ? AND " - + sessionAppCol + " = ?"; - preparedRemoveSql = _conn.prepareStatement(removeSql); - } - - preparedRemoveSql.setString(1, id); - preparedRemoveSql.setString(2, getName()); - preparedRemoveSql.execute(); + remove(id, _conn); // Break out after the finally block numberOfTries = 0; } catch (SQLException e) { @@ -740,6 +731,28 @@ public class JDBCStore extends StoreBase } /** + * Remove the Session with the specified session identifier from + * this Store, if present. If no such Session is present, this method + * takes no action. + * + * @param id Session identifier of the Session to be removed + * @param _conn open connection to be used + * @throws SQLException if an error occurs while talking to the database + */ + private void remove(String id, Connection _conn) throws SQLException { + if (preparedRemoveSql == null) { + String removeSql = "DELETE FROM " + sessionTable + + " WHERE " + sessionIdCol + " = ? AND " + + sessionAppCol + " = ?"; + preparedRemoveSql = _conn.prepareStatement(removeSql); + } + + preparedRemoveSql.setString(1, id); + preparedRemoveSql.setString(2, getName()); + preparedRemoveSql.execute(); + } + + /** * Remove all of the Sessions in this Store. * * @exception IOException if an input/output error occurs @@ -799,12 +812,12 @@ public class JDBCStore extends StoreBase return; } - // If sessions already exist in DB, remove and insert again. - // TODO: - // * Check if ID exists in database and if so use UPDATE. - remove(session.getIdInternal()); - try { + // If sessions already exist in DB, remove and insert again. + // TODO: + // * Check if ID exists in database and if so use UPDATE. + remove(session.getIdInternal(), _conn); + bos = new ByteArrayOutputStream(); oos = new ObjectOutputStream(new BufferedOutputStream(bos)); @@ -874,11 +887,13 @@ public class JDBCStore extends StoreBase * @return <code>Connection</code> if the connection succeeded */ protected Connection getConnection() { + Connection conn = null; try { - if (dbConnection == null || dbConnection.isClosed()) { + conn = open(); + if (conn == null || conn.isClosed()) { manager.getContainer().getLogger().info(sm.getString(getStoreName() + ".checkConnectionDBClosed")); - open(); - if (dbConnection == null || dbConnection.isClosed()) { + conn = open(); + if (conn == null || conn.isClosed()) { manager.getContainer().getLogger().info(sm.getString(getStoreName() + ".checkConnectionDBReOpenFail")); } } @@ -887,7 +902,7 @@ public class JDBCStore extends StoreBase ex.toString())); } - return dbConnection; + return conn; } /** @@ -916,8 +931,7 @@ public class JDBCStore extends StoreBase } if (dataSource != null) { - dbConnection = dataSource.getConnection(); - return dbConnection; + return dataSource.getConnection(); } // Instantiate our database driver if necessary @@ -1014,13 +1028,15 @@ public class JDBCStore extends StoreBase } /** - * Release the connection, not needed here since the - * connection is not associated with a connection pool. + * Release the connection, if it + * is associated with a connection pool. * * @param conn The connection to be released */ protected void release(Connection conn) { - // NOOP + if (dataSource != null) { + close(conn); + } } /** @@ -1033,8 +1049,10 @@ public class JDBCStore extends StoreBase @Override protected synchronized void startInternal() throws LifecycleException { - // Open connection to the database - this.dbConnection = getConnection(); + if (dataSource == null) { + // If not using a connection pool, open a connection to the database + this.dbConnection = getConnection(); + } super.startInternal(); } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1133134&r1=1133133&r2=1133134&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Tue Jun 7 19:28:42 2011 @@ -46,6 +46,11 @@ <subsection name="Catalina"> <changelog> <fix> + <bug>51264</bug>: Improve the previous fix for this issue by returning + the connection to the pool when not in use so it does not appear to be + an abandoned connection. Patch provided by Felix Schumacher. (markt) + </fix> + <fix> <bug>51324</bug>: Improve handling of exceptions when flushing the response buffer to ensure that the doFlush flag does not get stuck in the enabled state. Patch provided by Jeremy Norris. (markt) Modified: tomcat/trunk/webapps/docs/config/manager.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/manager.xml?rev=1133134&r1=1133133&r2=1133134&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/config/manager.xml (original) +++ tomcat/trunk/webapps/docs/config/manager.xml Tue Jun 7 19:28:42 2011 @@ -360,7 +360,10 @@ <p>Name of the JNDI resource for a JDBC DataSource-factory. If this option is given and a valid JDBC resource can be found, it will be used and any direct configuration of a JDBC connection via <code>connectionURL</code> - and <code>driverName</code> will be ignored.</p> + and <code>driverName</code> will be ignored. Since this code uses prepared + statements, you might want to configure pooled prepared statements as + shown in <a href="../jndi-resources-howto.html">the JNDI resources + HOW-TO</a>.</p> </attribute> <attribute name="driverName" required="true"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org