[
http://issues.apache.org/jira/browse/DERBY-210?page=comments#action_12366868 ]
Deepa Remesh commented on DERBY-210:
------------------------------------
Thanks Kathey for reading patch4.
The purpose of these lists do not seem to be same as what is indicated in the
code comments. In my work for DERBY-210, I found objects added to these lists
is one of the causes of memory leak. Solution that was suggested by you and
Bernt was to use weak references in these lists. In my patch proposal, I have
mentioned use of WeakHashMap instead of LinkedList.
I tried to see if I can remove these lists but thought it to be too disruptive
since the usage of these lists is not very clear. So I have opened DERBY-817
for further work on these lists. Please check DERBY-817 to see if it answers
some of your questions.
I am trying to summarize my understanding about these lists:
-------------------------------------------------
openStatements_ :
-------------------------------------------------
* What is it used for?
When Connection.close() is called, this list is checked to get a list of open
statements and statements are marked closed and removed from the list. This
happens in Connection.markStatementsClosed() method. They also are used in
reset method called by ClientPooledConnection to reset statements when re-using
the same physical connection.
* What is added and when?
All statement objects, except positioned update statements, are added to this
list. This happens in createStatement and prepare* methods in Connection class.
For positioned update statements, there is a comment in the code in
Connection.preparePositionedUpdateStatement:
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
// The positioned update statement is not added to the list of open
statements,
// because this would cause a java.util.ConcurrentModificationException
when
// iterating thru the list of open statements to call
completeRollback().
// An updatable result set is marked closed on a call to
completeRollback(),
// and would therefore need to close the positioned update statement
associated with the result set which would cause
// it to be removed from the open statements list. Resulting in
concurrent modification
// on the open statements list.
// Notice that ordinary Statement.closeX() is never called on the
positioned update statement,
// rather markClosed() is called to avoid trying to remove the
statement from the openStatements_ list.
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
* What is removed and when (finalize/close/some other time) ?
Currently, removal happens when 1) Connection is closed 2) Statement is
(explicitly) closed. 3) Code to *remove_from_list* is also there is the
finalize method call path. But this is a no-op since finalize() will get called
only when there are no references to objects in any lists.
-------------------------------------------------
CommitAndRollbackListeners_ :
-------------------------------------------------
* What is it used for?
Please see DERBY-817. It seems like this list is not needed at least for
purposes indicated by comment. This may need further investigation.
* What is added and when?
PreparedStatements, ResultSets and Lobs are added to this list from their
listenToUnitOfWork() methods.
* What is removed and when (finalize/close/some other time) ?
For all three types of objects, removal happens in their completeLocalCommit
and completeLocalRollback methods. For PreparedStatement and ResultSet, they
are also removed from this list when they are closed.
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
What patch4 (it changes finalization of statement objects) does to removal from
these lists :
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Removal of statement objects from these two lists is moved to markClosed
method. With respect to these lists, whatever was previosuly done by
markClosed() method remains unchanged. A new method markClosed(boolean
removeListener) is added to distinguish when the objects will be removed from
the list. markClosed(true) is only called from close() methods. In this case,
there is no change from previous behaviour since the following was previously
done in close() method:
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
markClosed();
connection_.openStatements_.remove(this);
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
and now it is replaced with
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
markClosed(true);
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Before this patch, finalize method code path also had *remove_from_list* call.
But this would never be visited. For finalize to be called, there should not be
any reference to the statement object from any lists. Applying the same logic,
in the patch, finalize just calls markClosed(), which does not have
*remove_from_list* call.
I hope this helps you in continuing to read the patch.
> Network Server will leak prepared statements if not explicitly closed by the
> user until the connection is closed
> ----------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-210
> URL: http://issues.apache.org/jira/browse/DERBY-210
> Project: Derby
> Type: Bug
> Components: Network Client
> Reporter: Kathey Marsden
> Assignee: Deepa Remesh
> Attachments: DOTS_ATCJ2_Derby-noPatch.png, DOTS_ATCJ2_Derby-withPatch.png,
> derby-210-patch1.diff, derby-210-patch2.diff, derby-210-patch2.status,
> derby-210-patch3.diff, derby-210-patch4-v2.diff, derby-210-patch4-v2.status,
> derby-210-v2-draft.diff, derby-210-v2-draft.status, derbyStress.java
>
> Network server will not garbage collect prepared statements that are not
> explicitly closed by the user. So a loop like this will leak.
> ...
> PreparedStatement ps;
> for (int i = 0 ; i < numPs; i++)
> {
> ps = conn.prepareStatement(selTabSql);
> rs =ps.executeQuery();
> while (rs.next())
> {
> rs.getString(1);
> }
> rs.close();
> // I'm a sloppy java programmer
> //ps.close();
> }
>
> To reproduce run the attached program
> java derbyStress
> Both client and server will grow until the connection is closed.
>
> It is likely that the fix for this will have to be in the client. The client
> does not send protocol to close the prepared statement, but rather reuses the
> PKGNAMCSN on the PRPSQLSTT request once the prepared statement has been
> closed. This is how the server knows to close the old statement and create a
> new one.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
