You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/06/07 21:28:43 UTC
svn commit: r1133134 - in /tomcat/trunk:
java/org/apache/catalina/session/JDBCStore.java webapps/docs/changelog.xml
webapps/docs/config/manager.xml
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org