Mailing-List: contact derby-commits-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Derby Development" <derby-dev@db.apache.org>
Message-ID: <20060117001115.31916.qmail@minotaur.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r369612 - in /db/derby/code/trunk/java:
 client/org/apache/derby/client/am/
 testing/org/apache/derbyTesting/functionTests/master/
 testing/org/apache/derbyTesting/functionTests/suites/
 testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/
Date: Tue, 17 Jan 2006 00:11:12 -0000
To: derby-commits@db.apache.org
From: kmarsden@apache.org

Author: kmarsden
Date: Mon Jan 16 16:10:58 2006
New Revision: 369612

URL: http://svn.apache.org/viewcvs?rev=369612&view=rev
Log:
DERBY-210 - Network Server will leak prepared statements if not explicitly closed by the user until the connection is closed


 I am uploading a combined patch 'derby-210.diff' which solves the memory leak. As Bryan suggested, I am uploading this patch and will open jira issues for other optimizations. Patch does the following:

* Eliminates the below references to PreparedStatement objects by using WeakHashMap instead of LinkedList. When there are no other references to the keys in a WeakHashMap, they will get removed from the map and can thus get garbage-collected. They do not have to wait till the Connection object is collected.
       - 'openStatements_' in org.apache.derby.client.am.Connection
       - 'CommitAndRollbackListeners_' in org.apache.derby.client.am.Connection

* Removes the list 'RollbackOnlyListeners_' since this is not being used.

* Updates the following comment for openStatements_:
// Since DERBY prepared statements must be re-prepared after a commit,
// then we must traverse this list after a commit and notify statements
// that they are now in an un-prepared state.
final java.util.LinkedList openStatements_ = new java.util.LinkedList();

In the code, I did not see this list being traversed after a commit to re-prepare statements. Also, I think this is not needed since Derby does not require re-prepare of statements after a commit. Currently, this list is used to close all open statements when the originating connection is closed.

* Removes all ResultSets from HashTable 'positionedUpdateCursorNameToResultSet_' in SectionManager. Only result sets of positioned update statements were being removed from this hashtable whereas all result sets were added. Because of this, client driver was holding on to result sets and statements even after rs.close() was called.

* Adds a test 'derbyStress.java' to jdbcapi suite. This test is based on the repro for this patch. Without this patch, it fails when run with client driver. Kathey had suggested in another mail that tests for client memory leak problems (DERBY-557, DERBY-210) can be added to same test. I did not see an existing test. So I created this new test. If DERBY-557 does not have a test, I think it can be added to this new test.

* Excludes the new test from running with jcc because jcc gives out of memory error.

* Creates 'derbyStress_app.properties' with following property 'jvmflags=-Xmx64M' to guarantee the test fails on all machines.

Successfully ran derbyall with Sun JDK 1.4.2 on Windows XP. Please take a look at this patch.


Contributed by Deepa Remesh


Added:
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/derbyStress.out   (with props)
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress.java   (with props)
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress_app.properties   (with props)
Modified:
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Lob.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/ResultSet.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/DerbyNet.exclude
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/jdbcapi.runall

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java?rev=369612&r1=369611&r2=369612&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java Mon Jan 16 16:10:58 2006
@@ -30,20 +30,21 @@
     public Agent agent_;
 
     public DatabaseMetaData databaseMetaData_;
-    // Since DERBY prepared statements must be re-prepared after a commit,
-    // then we must traverse this list after a commit and notify statements
-    // that they are now in an un-prepared state.
-    final java.util.LinkedList openStatements_ = new java.util.LinkedList();
+    
+    // WeakHashMap is used to store references so that the objects added to 
+    // the map can get garbage-collected without waiting for the Connection object.
+    
+    // When Connection.close() is called, this list is traversed and markClosed() 
+    // is called on all statements in this list.    
+    final java.util.WeakHashMap openStatements_ = new java.util.WeakHashMap();
 
-    // Some statuses of DERBY objects may be invalid on server either after only rollback
-    // or after both commit and rollback. For example,
+    // Some statuses of DERBY objects may be invalid on server after both 
+    // commit and rollback. For example,
     // (1) prepared statements need to be re-prepared
     //     after both commit and rollback
     // (2) result set will be unpositioned on server after both commit and rollback.
-    // If they only depend on rollback, they need to get on RollbackOnlyListeners_.
     // If they depend on both commit and rollback, they need to get on CommitAndRollbackListeners_.
-    final java.util.LinkedList RollbackOnlyListeners_ = new java.util.LinkedList();
-    final java.util.LinkedList CommitAndRollbackListeners_ = new java.util.LinkedList();
+    final java.util.WeakHashMap CommitAndRollbackListeners_ = new java.util.WeakHashMap();
     private SqlWarning warnings_ = null;
 
     // ------------------------properties set for life of connection--------------
@@ -394,7 +395,7 @@
         PreparedStatement ps = newPreparedStatement_(sql, java.sql.ResultSet.TYPE_FORWARD_ONLY, java.sql.ResultSet.CONCUR_READ_ONLY, resultSetHoldability_, java.sql.Statement.NO_GENERATED_KEYS, null);
         ps.isCatalogQuery_ = true;
         ps.prepare();
-        openStatements_.add(ps);
+        openStatements_.put(ps,null);
         return ps;
     }
 
@@ -737,7 +738,6 @@
         inUnitOfWork_ = false;
         markStatementsClosed();
         CommitAndRollbackListeners_.clear();
-        RollbackOnlyListeners_.clear();
         markClosed_();
     }
 
@@ -748,7 +748,8 @@
     }
 
     private void markStatementsClosed() {
-        for (java.util.ListIterator i = openStatements_.listIterator(); i.hasNext();) {
+    	java.util.Set keySet = openStatements_.keySet();
+        for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
             Statement stmt = (Statement) i.next();
             stmt.markClosed();
             i.remove();
@@ -756,13 +757,15 @@
     }
 
     private void writeCloseStatements() throws SqlException {
-        for (java.util.ListIterator i = openStatements_.listIterator(); i.hasNext();) {
+    	java.util.Set keySet = openStatements_.keySet();
+        for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
             ((Statement) i.next()).writeClose(false);  // false means don't permit auto-commits
         }
     }
 
     private void readCloseStatements() throws SqlException {
-        for (java.util.ListIterator i = openStatements_.listIterator(); i.hasNext();) {
+    	java.util.Set keySet = openStatements_.keySet();
+        for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
             ((Statement) i.next()).readClose(false);  // false means don't permit auto-commits
         }
     }
@@ -1220,7 +1223,7 @@
         resultSetConcurrency = downgradeResultSetConcurrency(resultSetConcurrency, resultSetType);
         Statement s = newStatement_(resultSetType, resultSetConcurrency, resultSetHoldability);
         s.cursorAttributesToSendOnPrepare_ = s.cacheCursorAttributesToSendOnPrepare();
-        openStatements_.add(s);
+        openStatements_.put(s,null);
         return s;
     }
 
@@ -1266,7 +1269,7 @@
         PreparedStatement ps = newPreparedStatement_(sql, resultSetType, resultSetConcurrency, resultSetHoldability, autoGeneratedKeys, columnNames);
         ps.cursorAttributesToSendOnPrepare_ = ps.cacheCursorAttributesToSendOnPrepare();
         ps.prepare();
-        openStatements_.add(ps);
+        openStatements_.put(ps,null);
         return ps;
     }
 
@@ -1304,7 +1307,7 @@
         CallableStatement cs = newCallableStatement_(sql, resultSetType, resultSetConcurrency, resultSetHoldability);
         cs.cursorAttributesToSendOnPrepare_ = cs.cacheCursorAttributesToSendOnPrepare();
         cs.prepare();
-        openStatements_.add(cs);
+        openStatements_.put(cs,null);
         return cs;
     }
 
@@ -1444,7 +1447,8 @@
     public abstract void readLocalCommit_() throws SqlException;
 
     public void completeLocalCommit() {
-        for (java.util.Iterator i = CommitAndRollbackListeners_.iterator(); i.hasNext();) {
+    	java.util.Set keySet = CommitAndRollbackListeners_.keySet();
+        for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
             UnitOfWorkListener listener = (UnitOfWorkListener) i.next();
             listener.completeLocalCommit(i);
         }
@@ -1459,11 +1463,8 @@
     // This is a client-side only operation.
     // This method will only throw an exception on bug check.
     public void completeLocalRollback() {
-        for (java.util.Iterator i = CommitAndRollbackListeners_.iterator(); i.hasNext();) {
-            UnitOfWorkListener listener = (UnitOfWorkListener) i.next();
-            listener.completeLocalRollback(i);
-        }
-        for (java.util.Iterator i = RollbackOnlyListeners_.iterator(); i.hasNext();) {
+    	java.util.Set keySet = CommitAndRollbackListeners_.keySet();
+        for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
             UnitOfWorkListener listener = (UnitOfWorkListener) i.next();
             listener.completeLocalRollback(i);
         }
@@ -1565,7 +1566,8 @@
         // Notice that these physical statements may not belong to this logical connection.
         // Iterate through the physical statements and re-enable them for reuse.
 
-        for (java.util.Iterator i = openStatements_.iterator(); i.hasNext();) {
+        java.util.Set keySet = openStatements_.keySet();
+        for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
             Object o = i.next();
             ((Statement) o).reset(recomputeFromDataSource);
 

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Lob.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/client/org/apache/derby/client/am/Lob.java?rev=369612&r1=369611&r2=369612&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Lob.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Lob.java Mon Jan 16 16:10:58 2006
@@ -64,7 +64,7 @@
     //-----------------------event callback methods-------------------------------
 
     public void listenToUnitOfWork() {
-        agent_.connection_.CommitAndRollbackListeners_.add(this);
+        agent_.connection_.CommitAndRollbackListeners_.put(this,null);
     }
 
     public void completeLocalCommit(java.util.Iterator listenerIterator) {

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java?rev=369612&r1=369611&r2=369612&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java Mon Jan 16 16:10:58 2006
@@ -1613,7 +1613,7 @@
     public void listenToUnitOfWork() {
         if (!listenToUnitOfWork_) {
             listenToUnitOfWork_ = true;
-            connection_.CommitAndRollbackListeners_.add(this);
+            connection_.CommitAndRollbackListeners_.put(this,null);
         }
     }
 

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/ResultSet.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/client/org/apache/derby/client/am/ResultSet.java?rev=369612&r1=369611&r2=369612&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/ResultSet.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/ResultSet.java Mon Jan 16 16:10:58 2006
@@ -3055,7 +3055,7 @@
     public void listenToUnitOfWork() {
         if (!listenToUnitOfWork_) {
             listenToUnitOfWork_ = true;
-            connection_.CommitAndRollbackListeners_.add(this);
+            connection_.CommitAndRollbackListeners_.put(this,null);
         }
     }
 

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java?rev=369612&r1=369611&r2=369612&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java Mon Jan 16 16:10:58 2006
@@ -2017,12 +2017,18 @@
     }
 
     // Only called on positioned upate statements
+    //Removing resultset from hashtable is called for all statements
     void resetCursorNameAndRemoveFromWhereCurrentOfMappings() {
         // Remove client/server cursorName -> ResultSet mapping from the hashtable.
         // If Statement.close() is called before ResultSet.close(), then statement_.section is null.
         if (section_ != null) {
             agent_.sectionManager_.removeCursorNameToResultSetMapping(cursorName_,
                     section_.getServerCursorNameForPositionedUpdate());
+            
+            // remove resultset mapping for other cursors (other than positioned
+            // update statements)
+            agent_.sectionManager_.removeCursorNameToResultSetMapping(cursorName_,
+                    section_.getServerCursorName());
 
             // Remove client and server cursorName -> QuerySection mapping from the hashtable
             // if one exists

Added: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/derbyStress.out
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/derbyStress.out?rev=369612&view=auto
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/derbyStress.out (added)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/derbyStress.out Mon Jan 16 16:10:58 2006
@@ -0,0 +1,4 @@
+Test derbyStress starting
+Testing with 5000 prepared statements
+PASS -- Prepared statement test
+Test derbyStress finished.
\ No newline at end of file

Propchange: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/derbyStress.out
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/DerbyNet.exclude
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/DerbyNet.exclude?rev=369612&r1=369611&r2=369612&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/DerbyNet.exclude (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/DerbyNet.exclude Mon Jan 16 16:10:58 2006
@@ -12,6 +12,7 @@
 # excluding jdbcapi/statementJdbc30.java - Client behaves differently. Need to look into this
 # excluding jdbcapi/holdCursorJava.java - JCC behaves differently with hold cursors.
 # excluding jdbcapi/dataSourceReference.java - client side only tests, tests all data sources
+# excluding jdbcapi/derbyStress.java - jcc runs out of memory with this test
 #           regardless of framework
 jdbcapi/resultsetStream.java
 lang/errorStream.java
@@ -28,3 +29,4 @@
 jdbcapi/statementJdbc30.java
 lang/holdCursorJava.java
 jdbcapi/dataSourceReference.java
+jdbcapi/derbyStress.java

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/jdbcapi.runall
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/jdbcapi.runall?rev=369612&r1=369611&r2=369612&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/jdbcapi.runall (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/suites/jdbcapi.runall Mon Jan 16 16:10:58 2006
@@ -1,6 +1,7 @@
 jdbcapi/bestrowidentifier.sql
 jdbcapi/characterStreams.java
 jdbcapi/checkDriver.java
+jdbcapi/derbyStress.java
 jdbcapi/nullSQLText.java
 jdbcapi/prepStmtMetaData.java
 jdbcapi/resultset.java

Added: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress.java?rev=369612&view=auto
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress.java (added)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress.java Mon Jan 16 16:10:58 2006
@@ -0,0 +1,120 @@
+/*
+
+Derby - Class org.apache.derbyTesting.functionTests.tests.jdbcapi.derbyStress
+
+Copyright 1999, 2005 The Apache Software Foundation or its licensors, as applicable.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+*/
+
+package org.apache.derbyTesting.functionTests.tests.jdbcapi;
+
+import java.sql.Connection;
+import java.sql.Statement;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+import org.apache.derby.tools.ij;
+import org.apache.derbyTesting.functionTests.util.TestUtil;
+
+public class derbyStress {
+	
+	private static String[] testObjects = {"table t1"};
+	
+	private static int numConn = 1;
+	private static int numRows = 100;
+	private static int numPreparedStmts = 5000; 
+
+	public static void main(String[] args) {
+		try {
+			System.out.println("Test derbyStress starting");
+
+			// use the ij utility to read the property file and
+			// make the initial connection.
+			ij.getPropertyArg(args);
+			Connection conn = null;
+			
+			for (int i = 0; i < numConn; i++) {
+				 conn = ij.startJBMS();
+				 System.out.println("Testing with " + numPreparedStmts + " prepared statements");
+				 prepStmtTest(conn, numRows, numPreparedStmts);
+				 System.out.println("PASS -- Prepared statement test");
+				 conn.close();
+			}
+			System.out.println("Test derbyStress finished.");
+		} catch (SQLException se) {
+			TestUtil.dumpSQLExceptions(se);
+		} catch (Throwable e) {
+			System.out.println("FAIL -- unexpected exception caught in main():\n");
+			System.out.println(e.getMessage());
+			e.printStackTrace();
+		}
+	}
+	
+	private static void createTables(Connection conn, int numRows) throws SQLException{
+		Statement stmt = conn.createStatement();
+		
+		TestUtil.cleanUpTest(stmt, testObjects);
+		
+		stmt.execute("create table t1 (lvc  LONG VARCHAR)");
+		stmt.close();
+		
+		String insertTabSql = "insert into t1 values(?)";
+		PreparedStatement ps = conn.prepareStatement(insertTabSql);
+		 for (int i = 0; i < numRows; i++)
+		 {
+			 ps.setString(1,"Hello" + i);
+			 ps.executeUpdate();
+		 }
+		 ps.close();
+	}
+	
+	// Tests prepared statements are not leaked if not explicitly closed by
+	// user (DERBY-210)
+	private static void prepStmtTest(Connection conn, int numRows, int numPreparedStmts) throws Exception
+	{
+		PreparedStatement ps = null;
+		ResultSet rs = null;
+		conn.setAutoCommit(false);
+		 
+		try {
+		
+			createTables(conn, numRows);
+			
+			String selTabSql = "select * from t1";
+			
+			for (int i = 0 ; i  < numPreparedStmts; i++)
+			{
+				ps = conn.prepareStatement(selTabSql);
+				rs = ps.executeQuery();
+
+				while (rs.next())
+				{
+					rs.getString(1);
+				}
+
+				rs.close();
+			
+				// Do not close the prepared statement
+				//ps.close();
+			}
+			conn.commit();
+		} 
+		catch (SQLException e) {
+			TestUtil.dumpSQLExceptions(e);
+			conn.rollback();
+		}
+	}
+}

Propchange: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress_app.properties
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress_app.properties?rev=369612&view=auto
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress_app.properties (added)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress_app.properties Mon Jan 16 16:10:58 2006
@@ -0,0 +1,2 @@
+usedefaults=true
+jvmflags=-Xmx64M
\ No newline at end of file

Propchange: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress_app.properties
------------------------------------------------------------------------------
    svn:eol-style = native