You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by km...@apache.org on 2006/02/25 04:57:53 UTC

svn commit: r380892 - in /db/derby/code/trunk/java/client/org/apache/derby/client/am: PreparedStatement.java Statement.java

Author: kmarsden
Date: Fri Feb 24 19:57:51 2006
New Revision: 380892

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

Contributed by Deepa Remesh

Changes the finalizer method in Statement and PreparedStatement to not send commits or CLSQRY to the server


n PreparedStatement class, the finalizer was calling closeX method, which was doing:
    * Call super.closeX() ---> Statement.closeX()
    * Cleanup parameter objects - parameterMetaData_, sql_, parameters_ array
    * Remove the PreparedStatement from connection_.CommitAndRollbackListeners_ list

   Changes done by patch:
    * Add a new method markClosed() which will free client-side resources.
    * The new method is named markClosed() to keep it uniform with naming convention in superclass.
    * This method is called from close() and finalize() methods.
    * markClosed() method will call super.markClosed() to perform cleanup of parent class. It will cleanup the objects specific to PreparedStatement, which are ParameterMetaData and parameters. It also removes the PreparedStatement form the list in Connection object.
    
2. In Statement class, the finalizer was calling closeX method, which was doing:
    * Close any open cursors for this statement on the server.
           - If result set is open on server, send CLSQRY to the server.
           - check if autocommit is required when closing result sets and flow a commit to server, if required
    * Call Statement.markClosed(), which does
             - Mark close the result sets on the client
             - If cursor name was set on the statement, remove it from Connection.clientCursorNameCache_
             - Call markClosed() on prepared statements for auto generated keys
             - Call markClosedOnServer(), which frees up the section. The freed section will be re-used by new statements.
    * Remove the Statement from Connection.openStatements_ list
    * Cleanup ResultSetMetaData

    Changes done by patch:
        * Move the cleanup of ResultSetMetaData and remove of Statement from Connection.openStatements_ list into markClosed() method. This will keep all client-side cleanup in markClosed().
        * Change the finalizer to just call markClosed(). This method frees up client-side resources and operates on synchronized collections. So I have removed the synchronize block from the finalizer.
     * The autocommit logic does not exist in the finalizer since only markClosed() is called from finalizer. This will avoid untimely commits which was causing the regression in the test lang/updatableResultSet.java 

-This line, and those below, will be ignored--

M    java/client/org/apache/derby/client/am/Statement.java
M    java/client/org/apache/derby/client/am/PreparedStatement.java

Modified:
    db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java

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=380892&r1=380891&r2=380892&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 Fri Feb 24 19:57:51 2006
@@ -219,18 +219,6 @@
         }
     }
 
-    protected void finalize() throws java.lang.Throwable {
-        if (agent_.loggingEnabled()) {
-            agent_.logWriter_.traceEntry(this, "finalize");
-        }
-        if (openOnClient_) {
-            synchronized (connection_) {
-                closeX();
-            }
-        }
-        super.finalize();
-    }
-
     // called immediately after the constructor by Connection prepare*() methods
     void prepare() throws SqlException {
         try {
@@ -2003,29 +1991,13 @@
         }
     }
 
-    public void close() throws SQLException {
-        try
-        {
-            synchronized (connection_) {
-                if (agent_.loggingEnabled()) {
-                    agent_.logWriter_.traceEntry(this, "close");
-                }
-                closeX();
-            }
-        }
-        catch ( SqlException se )
-        {
-            throw se.getSQLException();
-        }
-    }
-
-    // An untraced version of close()
-    public void closeX() throws SqlException {
-        if (!openOnClient_) {
-            return;
-        }
-        super.closeX();
-        if (parameterMetaData_ != null) {
+    /* (non-Javadoc)
+     * @see org.apache.derby.client.am.Statement#markClosed(boolean)
+     */
+    protected void markClosed(boolean removeListener){
+    	super.markClosed(removeListener);
+    	
+    	if (parameterMetaData_ != null) {
             parameterMetaData_.markClosed();
             parameterMetaData_ = null;
         }
@@ -2040,7 +2012,8 @@
         }
         parameters_ = null;
 
-        connection_.CommitAndRollbackListeners_.remove(this);
+        if(removeListener)
+        	connection_.CommitAndRollbackListeners_.remove(this);
     }
 
 }

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=380892&r1=380891&r2=380892&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 Fri Feb 24 19:57:51 2006
@@ -358,14 +358,19 @@
         generatedKeysColumnNames_ = columnNames;
     }
 
+    /* (non-Javadoc)
+     * @see java.lang.Object#finalize()
+     * 
+     * This method cleans up client-side resources by calling markClosed().
+     * It is different from close() method, which also does clean up on server.
+     * Changes done as part of DERBY-210. 
+     */
     protected void finalize() throws java.lang.Throwable {
         if (agent_.loggingEnabled()) {
             agent_.logWriter_.traceEntry(this, "finalize");
         }
         if (openOnClient_) {
-            synchronized (connection_) {
-                closeX();
-            }
+            markClosed();
         }
         super.finalize();
     }
@@ -475,7 +480,25 @@
         }
     }
 
-    // An untraced version of close()
+    /**
+     * An untraced version of <code>close</code>. This method cleans up 
+     * client-side resources and also sends commands to network server to 
+     * perform clean up. This should not be called in the finalizer. 
+     * Difference between <code>finalize</code> and <code>close</code> is
+     * that close method does these things additionally (Changes done as 
+     * part of DERBY-210):
+     * 1) Sends commands to the server to close the result sets.
+     * 2) Sends commands to the server to close the result sets of the 
+     * generated keys query.
+     * 3) Sends a commit if autocommit is on and it is appropriate.
+     * 4) Explicitly removes the statement from connection_.openStatements_ 
+     * and CommitAndRollbackListeners_ by passing true to markClosed.  
+     * 
+     * We may need to do 1) in finalizer too. This is being tracked in 
+     * DERBY-1021
+     * 
+     * @throws SqlException
+     */
     public void closeX() throws SqlException {
         if (!openOnClient_) {
             return;
@@ -491,13 +514,7 @@
                 flowCloseOutsideUOW();
             }
         } finally {
-            markClosed();
-            connection_.openStatements_.remove(this);
-        }
-        // push the mark close of rsmd into Statement.markClosed() method
-        if (resultSetMetaData_ != null) {
-            resultSetMetaData_.markClosed();
-            resultSetMetaData_ = null;
+            markClosed(true);
         }
     }
 
@@ -1633,7 +1650,39 @@
         }
     }
 
+    /**
+     * This method cleans up client-side resources held by this Statement. 
+     * The Statement will not be removed from the open statements list and 
+     * PreparedStatement will also not be removed from the commit and rollback 
+     * listeners list in <code>org.apache.derby.client.am.Connection</code>.
+     * 
+     * This method is called from:
+     * 1. finalize() - For the finaizer to be called, the Statement 
+     * should not have any references and so it should have been already 
+     * removed from the lists.  
+     * 
+     * 2. <code>org.apache.derby.client.am.Connection#markStatementsClosed</code> 
+     * This method explicitly removes the Statement from open statements list.
+     *  
+     * 3. To close positioned update statements - These statements are not
+     * added to the list of open statements.
+     */
     void markClosed() {
+    	markClosed(false);
+    }
+    
+    /**
+     * This method cleans up client-side resources held by this Statement. 
+     * If removeListener is true, the Statement is removed from open statements
+     * list and PreparedStatement is also removed from commit and rollback 
+     * listeners list. This is called from the close methods.
+     * 
+     * @param removeListener if true the Statement will be removed
+     * from the open statements list and PreparedStatement will also be removed
+     * from commit and rollback listeners list in 
+     * <code>org.apache.derby.client.am.Connection</code>.
+     */
+    void markClosed(boolean removeListener) {
         openOnClient_ = false;
         markResultSetsClosed();
         // in case a cursorName was set on the Statement but the Statement was
@@ -1643,6 +1692,15 @@
         removeClientCursorNameFromCache();
         markPreparedStatementForAutoGeneratedKeysClosed();
         markClosedOnServer();
+
+        // mark close ResultSetMetaData
+        if (resultSetMetaData_ != null) {
+            resultSetMetaData_.markClosed();
+            resultSetMetaData_ = null;
+        }
+        
+        if(removeListener)
+        	connection_.openStatements_.remove(this);
     }
 
     void markPreparedStatementForAutoGeneratedKeysClosed() {