[
http://issues.apache.org/jira/browse/DERBY-210?page=comments#action_12363439 ]
Deepa Remesh commented on DERBY-210:
------------------------------------
I looked into the failure in lang/updatableResultSet.java with derbynetclient
on JDK 1.5 and this is what I found:
This test was failing after my patch for derby-210 was committed. The failure
occurs in the following part of the test. I have included some of my thoughts
as comments starting with "// *****" in the below code:
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
System.out.println("Positive Test34 - in autocommit mode, check that updateRow
and deleteRow does not commit");
conn.setAutoCommit(true);
// First try deleteRow and updateRow on *first* row of result set
reloadData();
System.out.println(" Contents before changes to first row in RS:");
dumpRS(stmt.executeQuery("select * from t1"));
stmt = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_UPDATABLE);
rs = stmt.executeQuery("SELECT * FROM t1 FOR UPDATE");
rs.next();
rs.deleteRow();
conn.rollback();
rs.close();
System.out.println(" Make sure the contents of table are unchanged:");
dumpRS(stmt.executeQuery("select * from t1"));
stmt = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_UPDATABLE);
rs = stmt.executeQuery("SELECT * FROM t1 FOR UPDATE");
rs.next();
rs.updateInt(1,-rs.getInt(1));
rs.updateRow();
//
****************************************************************************************************************
// ************************************* A commit was occurring
here ******************************************
//
****************************************************************************************************************
conn.rollback();
rs.close();
System.out.println(" Make sure the contents of table are unchanged:");
dumpRS(stmt.executeQuery("select * from t1"));
//
***************************************************************************************************************
// ******************************* Test was failing since the
row gets changed due to the commit ***********
//
***************************************************************************************************************
// Now try the same on the *last* row in the result set
reloadData();
stmt = conn.createStatement();
rs = stmt.executeQuery("SELECT COUNT(*) FROM t1");
rs.next();
int count = rs.getInt(1);
rs.close();
System.out.println(" Contents before changes to last row in RS:");
dumpRS(stmt.executeQuery("select * from t1"));
stmt = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_UPDATABLE);
rs = stmt.executeQuery("SELECT * FROM t1 FOR UPDATE");
for (int j = 0; j < count; j++) {
rs.next();
}
rs.deleteRow();
conn.rollback();
rs.close();
System.out.println(" Make sure the contents of table are unchanged:");
dumpRS(stmt.executeQuery("select * from t1"));
stmt = conn.createStatement();
rs = stmt.executeQuery("SELECT COUNT(*) FROM t1");
rs.next();
count = rs.getInt(1);
rs.close();
stmt = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_UPDATABLE);
rs = stmt.executeQuery("SELECT * FROM t1 FOR UPDATE");
for (int j = 0; j < count; j++) {
rs.next();
}
rs.updateInt(1,-rs.getInt(1));
rs.updateRow();
//
*******************************************************************************************************************************
// ************* No commit was occurring here. This made me
think the commit is being sent from some other place
//
*******************************************************************************************************************************
conn.rollback();
rs.close();
System.out.println(" Make sure the contents of table are unchanged:");
dumpRS(stmt.executeQuery("select * from t1"));
//
****************************************************************************************************************
// ******************************* Test passes here, which
means the failure is sporadic *******************
//
****************************************************************************************************************
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
I could see in Network Server trace that it was getting a commit command
(RDBCMM) in between the rs.updateRow and conn.rollback() only in one particular
case. I tried to trace the client to see from where this commit was coming
from. But turning on client trace made this problem go away. I tried to create
a smaller repro for the problem but was not able to simulate this failure.
Also, the test was failing only on "Sun JDK 1.5.0_02". If I even slightly
change the order in the test, this problem goes away. So the failure was
sporadic but happens consistently with the updatableResultSet test on Sun JDK
1.5.0_02. I am glad we caught this problem on this jvm. Because after
debugging, I realize this can occur on any jvm/platform. It just depends on
when the garbage collector (GC) runs.
Since my patch changed when statements get garbage collected, I walked through
that path and found that it was the GC that was sending this commit to network
server. When a statement object gets ready for garbage collection, GC calls
it's finalize() method. The finalize() method in Statement class actually calls
closeX() which does the same things as Statement.close() method. When
autocommit is on, in some cases, this will call connection_.writeAutoCommit()
as shown by this code snippet from Statement.writeCloseResultSets. This causes
a commit to be sent to network server and if this happens at wrong time, the
output becomes erroneous.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
if (connection_.autoCommit_ && requiresAutocommit &&
isAutoCommittableStatement_) {
connection_.writeAutoCommit();
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
This problem will not occur without my patch because of this:
--------------------------------
Without my patch:
--------------------------------
Statements get free for garbage collection:
* ONLY after they are explicitly closed (Statement.close() has been called) or
the connection itself is closed.
AND
* when the user application has no more references to the Statement
In this case, when GC claims a statement object, it would have been closed
already. Because of this, when finalize() --> closeX() is called, openOnClient_
is false and hence finalize() method returns without calling closeX().
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
protected void finalize() throws java.lang.Throwable {
if (agent_.loggingEnabled()) {
agent_.logWriter_.traceEntry(this, "finalize");
}
if (openOnClient_) {
synchronized (connection_) {
closeX();
}
}
super.finalize();
}
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
--------------------------------
With my patch:
--------------------------------
Statements get free for garbage collection:
* when the user application has no more references to the Statement
which I think is the correct behavior.
In this case, when GC claims a statement object, openOnClient_ will be true if
no Statement.close() has been called previously. Because of this, finalize()
--> closeX() is called. And it goes through the entire chain of actions which
happen in closeX(). One of this is a call to connection_.writeAutoCommit()
which sends a RDBCMM to the server and server does a commit. By chance if the
commit message is recieved after the execution of a statement which is not
supposed to be autocommitted (like the updateRow statement), this causes
erroneous behavior.
I verified using printlns that this commit is getting sent from
Statement.writeCloseResultSets. Also ran the test after removing call to
closeX() from Statement.finalize(), in which case this problem goes away. This
call to closeX() seems to be the cause of the problem. I am yet to figure out
what subset of actions need to be done in finalize().
--------------------------------
Summary:
--------------------------------
I think my patch for derby-210 is doing the right thing by preventing statement
leaks. However, with my patch, some paths of code are getting visited for first
time from the GC thread. This is causing sporadic failures because the finalize
method in statement class (I think) does a lot more things than required, one
of them being sending commit message to the server. I think the statement
finalizer should not be sending anything to the network server, at least not at
connection level. I have to look some more into the finalizers in client
classes and what happens at server side before I can re-submit a patch for
this. I thought I'll post this to see what others think.
I would appreciate any feedback on this.
> 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: derby-210.diff, derby-210.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
