db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kmars...@apache.org
Subject svn commit: r384331 - in /db/derby/code/trunk/java: client/org/apache/derby/client/am/ testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/
Date Wed, 08 Mar 2006 21:16:50 GMT
Author: kmarsden
Date: Wed Mar  8 13:16:49 2006
New Revision: 384331

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


Contributed by Deepa Remesh

----------------------------------------------------------------
Summary of patch: 'derby-210-patch5-v1.diff'
-----------------------------------------------------------------
1. 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

2. 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.

3. 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.

4. Modifies the test jdbcapi/derbyStress.java to run the test for derby-210. The test was
checked in as patch2 but disabled for client framework.



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/tests/jdbcapi/derbyStress.java

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=384331&r1=384330&r2=384331&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 Wed Mar  8
13:16:49 2006
@@ -34,10 +34,15 @@
     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();
+    // DERBY-210 -  WeakHashMap is used to store references to objects to avoid
+    // memory leaks. 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.
+        
+    // In Connection.markStatementsClosed() method, this list is traversed to get a
+    // list of open statements, which are marked closed and removed from the list.
+    final java.util.WeakHashMap openStatements_ = new java.util.WeakHashMap();
 
     // Some statuses of DERBY objects may be invalid on server
     // after both commit and rollback. For example,
@@ -45,7 +50,7 @@
     //     after both commit and rollback
     // (2) result set will be unpositioned on server after both commit and rollback.
     // If they depend on both commit and rollback, they need to get on CommitAndRollbackListeners_.
-    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--------------
@@ -426,7 +431,7 @@
         PreparedStatement ps = newPreparedStatement_(sql, java.sql.ResultSet.TYPE_FORWARD_ONLY,
java.sql.ResultSet.CONCUR_READ_ONLY, holdability(), java.sql.Statement.NO_GENERATED_KEYS,
null);
         ps.isCatalogQuery_ = true;
         ps.prepare();
-        openStatements_.add(ps);
+        openStatements_.put(ps, null);
         return ps;
     }
 
@@ -825,7 +830,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();
@@ -833,13 +839,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
         }
     }
@@ -1462,7 +1470,7 @@
         }
         Statement s = newStatement_(resultSetType, resultSetConcurrency, resultSetHoldability);
         s.cursorAttributesToSendOnPrepare_ = s.cacheCursorAttributesToSendOnPrepare();
-        openStatements_.add(s);
+        openStatements_.put(s, null);
         return s;
     }
 
@@ -1515,7 +1523,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;
     }
 
@@ -1560,7 +1568,7 @@
         CallableStatement cs = newCallableStatement_(sql, resultSetType, resultSetConcurrency,
resultSetHoldability);
         cs.cursorAttributesToSendOnPrepare_ = cs.cacheCursorAttributesToSendOnPrepare();
         cs.prepare();
-        openStatements_.add(cs);
+        openStatements_.put(cs,null);
         return cs;
     }
 
@@ -1721,7 +1729,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);
         }
@@ -1736,7 +1745,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();)
{
+    	java.util.Set keySet = CommitAndRollbackListeners_.keySet();
+    	for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
             UnitOfWorkListener listener = (UnitOfWorkListener) i.next();
             listener.completeLocalRollback(i);
         }
@@ -1838,7 +1848,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=384331&r1=384330&r2=384331&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 Wed Mar  8 13:16:49
2006
@@ -66,7 +66,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=384331&r1=384330&r2=384331&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 Wed
Mar  8 13:16:49 2006
@@ -1887,7 +1887,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=384331&r1=384330&r2=384331&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 Wed Mar  8 13:16:49
2006
@@ -4157,7 +4157,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=384331&r1=384330&r2=384331&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 Wed Mar  8 13:16:49
2006
@@ -2320,13 +2320,22 @@
         return connection_;
     }
 
-    // Only called on positioned upate statements
+    // This was being called only on positioned update statements. When working 
+    // on DERBY-210, it was found that result sets of all statements (not just
+    // positioned update statements) get added to the table. So, this is called
+    // for all statements. Otherwise, this will cause memory leaks when statements
+    // are not explicitly closed in the application. 
     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) - DERBY-210
+            agent_.sectionManager_.removeCursorNameToResultSetMapping(cursorName_,
+                    section_.getServerCursorName());
 
             // Remove client and server cursorName -> QuerySection mapping from the hashtable
             // if one exists

Modified: 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=384331&r1=384330&r2=384331&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/derbyStress.java
Wed Mar  8 13:16:49 2006
@@ -88,9 +88,6 @@
 	// user (DERBY-210)
 	private static void prepStmtTest(Connection conn, int numRows, int numPreparedStmts) throws
Exception
 	{
-		// Don't run under DerbyNetClient until DERBY-210 is fixed
-		if (TestUtil.isDerbyNetClientFramework()) return;
-
 		PreparedStatement ps = null;
 		ResultSet rs = null;
 		conn.setAutoCommit(false);



Mime
View raw message