Derby
  1. Derby
  2. DERBY-1002

Check that DRDAStatement and DRDAResultSet states are reset when they are re-used

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.1.3.1, 10.2.1.6
    • Component/s: Network Server
    • Labels:
      None

      Description

      Network server re-uses DRDAStatement and DRDAResultSet objects when client sends a request with same section number. When re-using DRDAStatement, it's close() method is called which inturn calls close() method of DRDAResultSet. For re-use to work properly, we have to ensure the states of these objects are reset. This is not a bug but it is an area for possible improvements like:

      • The reset of all states are not in the close() methods. The states get re-initialized at different places in the code. Fo example, in case of DRDAResultSet, they get initialized in some other DRDAStatement methods - like addResultSet, setRsDefaultOptions, setOPNQRYOptions, setQueryOptions etc. It will be good to have all resets in one method.
      • The method name "close" is confusing since it is also called when objects get re-used. For clarity, it may be good to have a method named reset(). And then have the close method call reset.
      1. derby1002.java
        6 kB
        Deepa Remesh
      2. derby1002-patch1-draft1.diff
        7 kB
        Deepa Remesh
      3. derby1002-patch1-draft1.status
        0.2 kB
        Deepa Remesh
      4. derby1002-patch1-v1.diff
        7 kB
        Deepa Remesh
      5. derby1002-patch1-v1.status
        0.2 kB
        Deepa Remesh
      6. derby1002-patch2-v2.diff
        4 kB
        Deepa Remesh
      7. derby1002-patch2-v2.status
        0.1 kB
        Deepa Remesh

        Activity

        Hide
        Deepa Remesh added a comment -

        I had checked and thought the reset/re-initialization of DRDAStatement and DRDAResultSet states is happening correctly in the network server code. Hence I had marked this issue to be an improvement. I found I was wrong. Sorry about that.

        Things I had not understood correctly:

        1) DRDAStatement.rsClose() has this check: if (currentDrdaRs.getResultSet() == null) return;
        I had thought the condition will evaluate to true only when DRDAResultSet is constructed or after DRDAResultSet.close() has been called. So the DRDAResultSet would have been already reset whenever this condition is true. This is not correct.

        2) Fields like DRDAResultSet.qryclsimp get set only in OPNQRY path. However, writeQRYDTA method used for OPNQRY and EXCSQLSTT are same. Hence, in EXCSQLSTT path, it is possible for the query to use a left-over qryclsimp. Because of 1) above, if hasdata value is also not reset and is false, server will close a query wrongly (even when the query should not be closed implicitly and actually has more data). One possible error case is - A client which expects a QRYDTA as reply to CNTQRY may get a QRYNOPRM from the server.

        In short, this part of network server code is not correct and is causing bugs which are easy to repro with my patch for DERBY-210 but also occur with the current trunk. This issue blocks DERBY-210.

        I have a repro for a bug against the trunk which I will upload shortly with some more context.

        Show
        Deepa Remesh added a comment - I had checked and thought the reset/re-initialization of DRDAStatement and DRDAResultSet states is happening correctly in the network server code. Hence I had marked this issue to be an improvement. I found I was wrong. Sorry about that. Things I had not understood correctly: 1) DRDAStatement.rsClose() has this check: if (currentDrdaRs.getResultSet() == null) return; I had thought the condition will evaluate to true only when DRDAResultSet is constructed or after DRDAResultSet.close() has been called. So the DRDAResultSet would have been already reset whenever this condition is true. This is not correct. 2) Fields like DRDAResultSet.qryclsimp get set only in OPNQRY path. However, writeQRYDTA method used for OPNQRY and EXCSQLSTT are same. Hence, in EXCSQLSTT path, it is possible for the query to use a left-over qryclsimp. Because of 1) above, if hasdata value is also not reset and is false, server will close a query wrongly (even when the query should not be closed implicitly and actually has more data). One possible error case is - A client which expects a QRYDTA as reply to CNTQRY may get a QRYNOPRM from the server. In short, this part of network server code is not correct and is causing bugs which are easy to repro with my patch for DERBY-210 but also occur with the current trunk. This issue blocks DERBY-210 . I have a repro for a bug against the trunk which I will upload shortly with some more context.
        Hide
        Deepa Remesh added a comment -

        Attaching a repro 'derby1002.java' which shows how queries can get closed on the server unexpectedly which leads to protocol exception. To run the repro:

        1. Start network server on port 2222.
        2. Run the command: java derby1002
        3. Output should be:

        ******************************************
        testImplicitClose
        ******************************************
        FAILED (no exception thrown)

        ******************************************
        jira491Test
        ******************************************
        JIRA-491 FAILURE:
        Client sends CNTQRY and expects QRYDTA. Server sends QRYNOPRM because the query has been implicitly
        closed
        Caught Exception:
        java.sql.SQLException: Execution failed due to a distribution protocol error that caused deallocatio
        n of the conversation. The identified cursor is not open.
        at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:285)
        at org.apache.derby.client.am.ResultSet.next(ResultSet.java:269)
        at derby1002.jira491Test(derby1002.java:123)
        at derby1002.main(derby1002.java:26)
        Caused by: org.apache.derby.client.am.DisconnectException: Execution failed due to a distribution pr
        otocol error that caused deallocation of the conversation. The identified cursor is not open.
        at org.apache.derby.client.net.NetResultSetReply.parseQRYNOPRM(NetResultSetReply.java:331)
        at org.apache.derby.client.net.NetResultSetReply.parseFetchError(NetResultSetReply.java:226)

        at org.apache.derby.client.net.NetResultSetReply.parseCNTQRYreply(NetResultSetReply.java:177
        )
        at org.apache.derby.client.net.NetResultSetReply.readFetch(NetResultSetReply.java:38)
        at org.apache.derby.client.net.ResultSetReply.readFetch(ResultSetReply.java:40)
        at org.apache.derby.client.net.NetResultSet.readFetch_(NetResultSet.java:181)
        at org.apache.derby.client.am.ResultSet.flowFetch(ResultSet.java:4034)
        at org.apache.derby.client.net.NetCursor.completeSplitRow(NetCursor.java:1057)
        at org.apache.derby.client.net.NetCursor.skipFdocaBytes(NetCursor.java:537)
        at org.apache.derby.client.net.NetCursor.calculateColumnOffsetsForRow_(NetCursor.java:251)
        at org.apache.derby.client.am.Cursor.stepNext(Cursor.java:172)
        at org.apache.derby.client.am.Cursor.next(Cursor.java:185)
        at org.apache.derby.client.am.ResultSet.nextX(ResultSet.java:293)
        at org.apache.derby.client.am.ResultSet.next(ResultSet.java:260)
        ... 2 more

        I have taken parts of tests for jira 821 and jira 491 from lang/procedure.java to create the repro.

        --------------------------------------------------------
        Context:
        --------------------------------------------------------
        After Kathey suggested to separate out finalizer changes in DERBY-210, I updated my workpace and I was running derbynetclientmats suite in a loop with my patches. I was seeing a new intermittent failure in lang/procedure.java, which was not there last week. The failure was related to implicit closing of result sets and hence seeing it this week. Here are the two failures I see on repeated runs of this test with my patches. Sometimes I get either one of these failures or none:

        1) Failure in testImplicitClose(). This tests to see that the resultsets opened by EXCSQLSTT do not get implicitly closed. However, implicit close happens because stmt.hasdata() is evaluating to false at the end of writeQRYDTA. And the result set's qryclsimp value is also left over from a previous OPNQRY.

        942 del
        < testImplicitClose(): PASSED
        942 add
        > testImplicitClose(): FAILED (no exception thrown)
        Test Failed.

        2) In this failure also, implicit close happens because stmt.hasdata() is evaluating to false at the end of writeQRYDTA

            • Start: procedure jdk1.5.0_02 DerbyNetClient 2006-02-21 23:36:42 ***
              940 del
              < JIRA-491 Successful.
              941 del
              < JIRA-492 successful – no hang!
              942 del
              < testImplicitClose(): PASSED
              942 add
              > JIRA-491 FAILURE: Caught Exception:
              > java.sql.SQLException: Execution failed due to a distribution protocol error that caused deallocation of the conversation. The identified cursor is not open.
              > Caused by: org.apache.derby.client.am.DisconnectException: Execution failed due to a distribution protocol error that caused deallocation of the conversation. The identified cursor is not open.
              > ... 3 more
              > JIRA-492: FAILURE in data setup:
              > java.sql.SQLException: Invalid operation: statement closed
              > Caused by: org.apache.derby.client.am.SqlException: Invalid operation: statement closed
              > ... 3 more
              > JIRA-492: FAILURE in procedure creation:
              > java.sql.SQLException: Invalid operation: statement closed
              > Caused by: org.apache.derby.client.am.SqlException: Invalid operation: statement closed
              > ... 3 more
              > ERROR (no SQLState): invalid operation: connection closed
              > java.sql.SQLException: invalid operation: connection closed
              > Caused by: org.apache.derby.client.am.SqlException: invalid operation: connection closed
              > ... 3 more
              Test Failed.

        On looking at these, I found there could still be problems in the network server code where DRDAStatement and DRDAResultSet get re-used. Also, it seemed to me these failures are not related to my patch and should happen without my patch. I was able to create a repro using parts of the procedure test and creating/closing statements at right time to provoke this bug without my patch.

        --------------------------------------------------------
        Details of what repro does:
        --------------------------------------------------------
        1) Run a select statement so that OPNQRY will be sent to server and qryclsimp will be set to CodePoint.QRYCLSIMP_YES
        2) Close the above statement so that network server will re-use its DRDAStatement for the next statement.
        3) Create a callabale statement and execute a procedure and get all data so that hasdata for the DRDAResultSet will be false.
        4) Close the above statement so that network server will re-use its DRDAStatement for the next statement.
        5a) Run testImplicitClose() from procedure test. This test checks that result sets do not get implicitly closed when using EXCSQLSTT. But this will fail since the DRDAResultSet has wrong states and the 'writeQRYDTA' method used by OPNQRYRM and EXCSQLSTT commands is the same.
        5b) Run the test for jira 491. The QRYDTA will be split and hence hasdata must be true at the time of sending first QRYDTA. However, hasdata evaluates to false and hence server closes the result set. Client sends CNTQRY to get the next QRYDTA. Since server has closed the query, it sends back QRYNOPRM which causes a protocol exception.

        --------------------------------------------------------
        Summary:
        --------------------------------------------------------
        Network server code should be changed and reviewed thoroughly to see that the re-use of objects happens correctly.

        Show
        Deepa Remesh added a comment - Attaching a repro 'derby1002.java' which shows how queries can get closed on the server unexpectedly which leads to protocol exception. To run the repro: 1. Start network server on port 2222. 2. Run the command: java derby1002 3. Output should be: ****************************************** testImplicitClose ****************************************** FAILED (no exception thrown) ****************************************** jira491Test ****************************************** JIRA-491 FAILURE: Client sends CNTQRY and expects QRYDTA. Server sends QRYNOPRM because the query has been implicitly closed Caught Exception: java.sql.SQLException: Execution failed due to a distribution protocol error that caused deallocatio n of the conversation. The identified cursor is not open. at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:285) at org.apache.derby.client.am.ResultSet.next(ResultSet.java:269) at derby1002.jira491Test(derby1002.java:123) at derby1002.main(derby1002.java:26) Caused by: org.apache.derby.client.am.DisconnectException: Execution failed due to a distribution pr otocol error that caused deallocation of the conversation. The identified cursor is not open. at org.apache.derby.client.net.NetResultSetReply.parseQRYNOPRM(NetResultSetReply.java:331) at org.apache.derby.client.net.NetResultSetReply.parseFetchError(NetResultSetReply.java:226) at org.apache.derby.client.net.NetResultSetReply.parseCNTQRYreply(NetResultSetReply.java:177 ) at org.apache.derby.client.net.NetResultSetReply.readFetch(NetResultSetReply.java:38) at org.apache.derby.client.net.ResultSetReply.readFetch(ResultSetReply.java:40) at org.apache.derby.client.net.NetResultSet.readFetch_(NetResultSet.java:181) at org.apache.derby.client.am.ResultSet.flowFetch(ResultSet.java:4034) at org.apache.derby.client.net.NetCursor.completeSplitRow(NetCursor.java:1057) at org.apache.derby.client.net.NetCursor.skipFdocaBytes(NetCursor.java:537) at org.apache.derby.client.net.NetCursor.calculateColumnOffsetsForRow_(NetCursor.java:251) at org.apache.derby.client.am.Cursor.stepNext(Cursor.java:172) at org.apache.derby.client.am.Cursor.next(Cursor.java:185) at org.apache.derby.client.am.ResultSet.nextX(ResultSet.java:293) at org.apache.derby.client.am.ResultSet.next(ResultSet.java:260) ... 2 more I have taken parts of tests for jira 821 and jira 491 from lang/procedure.java to create the repro. -------------------------------------------------------- Context: -------------------------------------------------------- After Kathey suggested to separate out finalizer changes in DERBY-210 , I updated my workpace and I was running derbynetclientmats suite in a loop with my patches. I was seeing a new intermittent failure in lang/procedure.java, which was not there last week. The failure was related to implicit closing of result sets and hence seeing it this week. Here are the two failures I see on repeated runs of this test with my patches. Sometimes I get either one of these failures or none: 1) Failure in testImplicitClose(). This tests to see that the resultsets opened by EXCSQLSTT do not get implicitly closed. However, implicit close happens because stmt.hasdata() is evaluating to false at the end of writeQRYDTA. And the result set's qryclsimp value is also left over from a previous OPNQRY. 942 del < testImplicitClose(): PASSED 942 add > testImplicitClose(): FAILED (no exception thrown) Test Failed. 2) In this failure also, implicit close happens because stmt.hasdata() is evaluating to false at the end of writeQRYDTA Start: procedure jdk1.5.0_02 DerbyNetClient 2006-02-21 23:36:42 *** 940 del < JIRA-491 Successful. 941 del < JIRA-492 successful – no hang! 942 del < testImplicitClose(): PASSED 942 add > JIRA-491 FAILURE: Caught Exception: > java.sql.SQLException: Execution failed due to a distribution protocol error that caused deallocation of the conversation. The identified cursor is not open. > Caused by: org.apache.derby.client.am.DisconnectException: Execution failed due to a distribution protocol error that caused deallocation of the conversation. The identified cursor is not open. > ... 3 more > JIRA-492: FAILURE in data setup: > java.sql.SQLException: Invalid operation: statement closed > Caused by: org.apache.derby.client.am.SqlException: Invalid operation: statement closed > ... 3 more > JIRA-492: FAILURE in procedure creation: > java.sql.SQLException: Invalid operation: statement closed > Caused by: org.apache.derby.client.am.SqlException: Invalid operation: statement closed > ... 3 more > ERROR (no SQLState): invalid operation: connection closed > java.sql.SQLException: invalid operation: connection closed > Caused by: org.apache.derby.client.am.SqlException: invalid operation: connection closed > ... 3 more Test Failed. On looking at these, I found there could still be problems in the network server code where DRDAStatement and DRDAResultSet get re-used. Also, it seemed to me these failures are not related to my patch and should happen without my patch. I was able to create a repro using parts of the procedure test and creating/closing statements at right time to provoke this bug without my patch. -------------------------------------------------------- Details of what repro does: -------------------------------------------------------- 1) Run a select statement so that OPNQRY will be sent to server and qryclsimp will be set to CodePoint.QRYCLSIMP_YES 2) Close the above statement so that network server will re-use its DRDAStatement for the next statement. 3) Create a callabale statement and execute a procedure and get all data so that hasdata for the DRDAResultSet will be false. 4) Close the above statement so that network server will re-use its DRDAStatement for the next statement. 5a) Run testImplicitClose() from procedure test. This test checks that result sets do not get implicitly closed when using EXCSQLSTT. But this will fail since the DRDAResultSet has wrong states and the 'writeQRYDTA' method used by OPNQRYRM and EXCSQLSTT commands is the same. 5b) Run the test for jira 491. The QRYDTA will be split and hence hasdata must be true at the time of sending first QRYDTA. However, hasdata evaluates to false and hence server closes the result set. Client sends CNTQRY to get the next QRYDTA. Since server has closed the query, it sends back QRYNOPRM which causes a protocol exception. -------------------------------------------------------- Summary: -------------------------------------------------------- Network server code should be changed and reviewed thoroughly to see that the re-use of objects happens correctly.
        Hide
        Deepa Remesh added a comment -

        Attaching a draft patch 'derby1002-patch1-draft1.diff'. This patch is only for review.

        Summary of patch:

        • Adds reset() methods to DRDAStatement and DRDAResultSet objects. The purpose of reset method is to reset the states of all variables so that the objects can be re-used and will not have left-over states.
        • In case of DRDAStatement, the following variables are not reset:
          + * 1. database - This variable gets initialized in the constructor and by
          + * call to setDatabase.
          + * 2. members which get initialized in setPkgnamcsn (pkgnamcsn, pkgcnstkn,
          + * pkgid, pkgsn, isolationLevel, cursorName). pkgnamcsn is the key used to
          + * find if the DRDAStatement can be re-used. Hence its value will not change
          + * when the object is re-used.
        • close() methods are changed to only close and dereference objects.
        • DRDAStatement.rsClose() method is not used in close() or reset(). This method has some checks which were causing the method to return without resetting currentDrdaRs. Now, close() calls currentDrdaRs.close() and reset() calls currentDrdaRs.reset().
        • In Database.newDrdaStatement, close() method is called followed by reset() when the server finds the statement can be re-used.

        Initially, I was thinking reset should also call close methods for the jdbc objects instead of just resetting them to null so that the jdbc objects get cleaned up properly if just reset() is called. Any comments on how it is done now? I was also thinking about calling reset at places where DRDAStatement.initialize is currently called to re-use default statements. On looking more into the code, I find default statements are different in the way they get initialized and used. e.g: stmt variable used in default statement is created only once in Database.makeConnection. I have to see the usage of default statements to see what exactly it means to re-initialize them.

        There are some TODO items identified in discussion with Knut and Kathey in the following mail thread:
        http://www.nabble.com/-DRDA-Question-about-DRDAStatement.initialize%28%29-method-t1177861.html

        TODOs:

        • Add the repro to test suite.
        • pkgcnstkn, pkgid, pkgsn variables can be removed from DRDAStatement since these are derived from pkgnamcsn object.
        • Look into what is required by initialize() of default statement. Currently, initialize just calls setTypDefValues().

        I ran my repro with this patch. derbyall is not complete. I will update with results of test run later. Please take a look at this patch. Thanks.

        Show
        Deepa Remesh added a comment - Attaching a draft patch 'derby1002-patch1-draft1.diff'. This patch is only for review. Summary of patch: Adds reset() methods to DRDAStatement and DRDAResultSet objects. The purpose of reset method is to reset the states of all variables so that the objects can be re-used and will not have left-over states. In case of DRDAStatement, the following variables are not reset: + * 1. database - This variable gets initialized in the constructor and by + * call to setDatabase. + * 2. members which get initialized in setPkgnamcsn (pkgnamcsn, pkgcnstkn, + * pkgid, pkgsn, isolationLevel, cursorName). pkgnamcsn is the key used to + * find if the DRDAStatement can be re-used. Hence its value will not change + * when the object is re-used. close() methods are changed to only close and dereference objects. DRDAStatement.rsClose() method is not used in close() or reset(). This method has some checks which were causing the method to return without resetting currentDrdaRs. Now, close() calls currentDrdaRs.close() and reset() calls currentDrdaRs.reset(). In Database.newDrdaStatement, close() method is called followed by reset() when the server finds the statement can be re-used. Initially, I was thinking reset should also call close methods for the jdbc objects instead of just resetting them to null so that the jdbc objects get cleaned up properly if just reset() is called. Any comments on how it is done now? I was also thinking about calling reset at places where DRDAStatement.initialize is currently called to re-use default statements. On looking more into the code, I find default statements are different in the way they get initialized and used. e.g: stmt variable used in default statement is created only once in Database.makeConnection. I have to see the usage of default statements to see what exactly it means to re-initialize them. There are some TODO items identified in discussion with Knut and Kathey in the following mail thread: http://www.nabble.com/-DRDA-Question-about-DRDAStatement.initialize%28%29-method-t1177861.html TODOs: Add the repro to test suite. pkgcnstkn, pkgid, pkgsn variables can be removed from DRDAStatement since these are derived from pkgnamcsn object. Look into what is required by initialize() of default statement. Currently, initialize just calls setTypDefValues(). I ran my repro with this patch. derbyall is not complete. I will update with results of test run later. Please take a look at this patch. Thanks.
        Hide
        Deepa Remesh added a comment -

        Attaching 'derby1002-patch1-v1.diff'. No changes from the draft patch. Ran derbyall with the patch (3 failures also seen in nightlies). I have also run derbynetclientmats 10 times in a loop with my patch for derby-210 since the problem was found there.

        Show
        Deepa Remesh added a comment - Attaching 'derby1002-patch1-v1.diff'. No changes from the draft patch. Ran derbyall with the patch (3 failures also seen in nightlies). I have also run derbynetclientmats 10 times in a loop with my patch for derby-210 since the problem was found there.
        Hide
        Deepa Remesh added a comment -

        Attaching a patch for the test 'derby1002-patch2-v1.diff'. This patch adds the test in the repro to lang/procedure.java. This patch has to be committed only after patch1 is reviewed and committed.

        The addtion to the test is a method which creates and uses statements in such a way as to provoke re-use of statements and result sets on network server. Before pacth1, re-use happens incorrectly and this causes the server to close queries wrongly. Without patch1, lang/procedure.java fails with DerbyNetClient framework. With patch1, lang/procedure.java passes with all three frameworks. Tests run with Sun JDK1.4.2 on Windows XP.

        Please look at patch1 and patch2.

        NOTE: Patch2 can be committed only after patch1 has been reviewed and committed

        Show
        Deepa Remesh added a comment - Attaching a patch for the test 'derby1002-patch2-v1.diff'. This patch adds the test in the repro to lang/procedure.java. This patch has to be committed only after patch1 is reviewed and committed. The addtion to the test is a method which creates and uses statements in such a way as to provoke re-use of statements and result sets on network server. Before pacth1, re-use happens incorrectly and this causes the server to close queries wrongly. Without patch1, lang/procedure.java fails with DerbyNetClient framework. With patch1, lang/procedure.java passes with all three frameworks. Tests run with Sun JDK1.4.2 on Windows XP. Please look at patch1 and patch2. NOTE: Patch2 can be committed only after patch1 has been reviewed and committed
        Hide
        Deepa Remesh added a comment -

        Attaching 'derby1002-patch2-v2.diff' for Bryans' comments in the following derby-dev mail:
        http://www.nabble.com/Re%3A-jira-Updated%3A-%28DERBY-1002%29-Check-that-DRDAStatement-and-DRDAResultSet-states-are-reset-when-they-are-re-used-p3135530.html

        This patch modifies only one file: lang/procedure.java. With this patch, I have re-run lang/procedure.java.

        Please take a look at following two patches - patch 1 contains code changes and patch 2 contains tests. These have to be committed together.

        ------------------------------------------
        1. derby1002-patch1-v1
        ------------------------------------------

        • Adds reset() methods to DRDAStatement and DRDAResultSet objects. The purpose of reset method is to reset the states of all variables so that the objects can be re-used and will not have left-over states.
        • In case of DRDAStatement, the following variables are not reset:
          + * 1. database - This variable gets initialized in the constructor and by
          + * call to setDatabase.
          + * 2. members which get initialized in setPkgnamcsn (pkgnamcsn, pkgcnstkn,
          + * pkgid, pkgsn, isolationLevel, cursorName). pkgnamcsn is the key used to
          + * find if the DRDAStatement can be re-used. Hence its value will not change
          + * when the object is re-used.
        • close() methods are changed to only close and dereference objects.
        • DRDAStatement.rsClose() method is not used in close() or reset(). This method has some checks which were causing the method to return without resetting currentDrdaRs. Now, close() calls currentDrdaRs.close() and reset() calls currentDrdaRs.reset().
        • In Database.newDrdaStatement, close() method is called followed by reset() when the server finds the statement can be re-used.

        ------------------------------------------
        2. derby1002-patch2-v2
        ------------------------------------------
        Modifies test lang/procedure.java. Adds a method 'setupStatementReuse' which creates and uses statements in such a way as to provoke re-use of statements and result sets on network server. This method is called from tests for jira-491 and 'testImplicitClose'.

        ------------------------------------------------------------------------------------
        Remaining TODOs for DERBY-1002
        ------------------------------------------------------------------------------------

        • pkgcnstkn, pkgid, pkgsn variables can be removed from DRDAStatement since these are derived from pkgnamcsn object.
        • Look into what is required by initialize() of default statement. Currently, initialize just calls setTypDefValues(). Once the purpose of this method is confirmed, we may need to modify the comments at places it is currently called.

        NOTE: patch 1 contains code changes and patch 2 contains tests. These have to be committed together.

        Show
        Deepa Remesh added a comment - Attaching 'derby1002-patch2-v2.diff' for Bryans' comments in the following derby-dev mail: http://www.nabble.com/Re%3A-jira-Updated%3A-%28DERBY-1002%29-Check-that-DRDAStatement-and-DRDAResultSet-states-are-reset-when-they-are-re-used-p3135530.html This patch modifies only one file: lang/procedure.java. With this patch, I have re-run lang/procedure.java. Please take a look at following two patches - patch 1 contains code changes and patch 2 contains tests. These have to be committed together. ------------------------------------------ 1. derby1002-patch1-v1 ------------------------------------------ Adds reset() methods to DRDAStatement and DRDAResultSet objects. The purpose of reset method is to reset the states of all variables so that the objects can be re-used and will not have left-over states. In case of DRDAStatement, the following variables are not reset: + * 1. database - This variable gets initialized in the constructor and by + * call to setDatabase. + * 2. members which get initialized in setPkgnamcsn (pkgnamcsn, pkgcnstkn, + * pkgid, pkgsn, isolationLevel, cursorName). pkgnamcsn is the key used to + * find if the DRDAStatement can be re-used. Hence its value will not change + * when the object is re-used. close() methods are changed to only close and dereference objects. DRDAStatement.rsClose() method is not used in close() or reset(). This method has some checks which were causing the method to return without resetting currentDrdaRs. Now, close() calls currentDrdaRs.close() and reset() calls currentDrdaRs.reset(). In Database.newDrdaStatement, close() method is called followed by reset() when the server finds the statement can be re-used. ------------------------------------------ 2. derby1002-patch2-v2 ------------------------------------------ Modifies test lang/procedure.java. Adds a method 'setupStatementReuse' which creates and uses statements in such a way as to provoke re-use of statements and result sets on network server. This method is called from tests for jira-491 and 'testImplicitClose'. ------------------------------------------------------------------------------------ Remaining TODOs for DERBY-1002 ------------------------------------------------------------------------------------ pkgcnstkn, pkgid, pkgsn variables can be removed from DRDAStatement since these are derived from pkgnamcsn object. Look into what is required by initialize() of default statement. Currently, initialize just calls setTypDefValues(). Once the purpose of this method is confirmed, we may need to modify the comments at places it is currently called. NOTE: patch 1 contains code changes and patch 2 contains tests. These have to be committed together.
        Hide
        Deepa Remesh added a comment -

        There are two minor todo items for this issue. I am not working on them currently, hence unassigning myself.

        Show
        Deepa Remesh added a comment - There are two minor todo items for this issue. I am not working on them currently, hence unassigning myself.
        Hide
        Deepa Remesh added a comment -

        Unchecking patch available check box as the patches submitted so far for this issue were committed. Also, this issue no longer blocks DERBY-210. I do not see a way how I can "unlink" these issues in JIRA.

        Show
        Deepa Remesh added a comment - Unchecking patch available check box as the patches submitted so far for this issue were committed. Also, this issue no longer blocks DERBY-210 . I do not see a way how I can "unlink" these issues in JIRA.
        Hide
        Deepa Remesh added a comment -

        Just after posting this, I found I can delete links by using "Manage Links". I have removed the links.

        Show
        Deepa Remesh added a comment - Just after posting this, I found I can delete links by using "Manage Links". I have removed the links.
        Hide
        Deepa Remesh added a comment -

        Patch 1 and 2 have been committed to trunk as well as 10.1 branch.

        Show
        Deepa Remesh added a comment - Patch 1 and 2 have been committed to trunk as well as 10.1 branch.
        Hide
        Andrew McIntyre added a comment -

        This has been partially fixed in 10.1.3. I'm thinking the right thing to do is mark it affects 10.1.3, then move the FixIn for the issue out to 10.1.4 and make a note of the partial fix in the release notes. Unless anyone has any objections, that's the course I'll take when putting together the release notes.

        Show
        Andrew McIntyre added a comment - This has been partially fixed in 10.1.3. I'm thinking the right thing to do is mark it affects 10.1.3, then move the FixIn for the issue out to 10.1.4 and make a note of the partial fix in the release notes. Unless anyone has any objections, that's the course I'll take when putting together the release notes.
        Hide
        Kathey Marsden added a comment -

        Deepa said:

        Patch 1 and 2 have been committed to trunk as well as 10.1 branch.
        Andrew said:
        This has been partially fixed in 10.1.3.

        What hasn't gone to 10.1.x?

        Show
        Kathey Marsden added a comment - Deepa said: Patch 1 and 2 have been committed to trunk as well as 10.1 branch. Andrew said: This has been partially fixed in 10.1.3. What hasn't gone to 10.1.x?
        Hide
        Deepa Remesh added a comment -

        The actual issue reported in this JIRA is fixed. There were some code cleanup TODOs as noted in previous comment:

        • pkgcnstkn, pkgid, pkgsn variables can be removed from DRDAStatement since these are derived from pkgnamcsn object.
        • Look into what is required by initialize() of default statement. Currently, initialize just calls setTypDefValues(). Once the purpose of this method is confirmed, we may need to modify the comments at places it is currently called.

        I had not resolved this issue in trunk or 10.1 because of these TODO items. To avoid confusion, I will mark this issue fixed and open a minor issue for code cleanup.

        Show
        Deepa Remesh added a comment - The actual issue reported in this JIRA is fixed. There were some code cleanup TODOs as noted in previous comment: pkgcnstkn, pkgid, pkgsn variables can be removed from DRDAStatement since these are derived from pkgnamcsn object. Look into what is required by initialize() of default statement. Currently, initialize just calls setTypDefValues(). Once the purpose of this method is confirmed, we may need to modify the comments at places it is currently called. I had not resolved this issue in trunk or 10.1 because of these TODO items. To avoid confusion, I will mark this issue fixed and open a minor issue for code cleanup.
        Hide
        Deepa Remesh added a comment -

        Resolved by svn revisions 381937 in trunk (10.2) and 408612 in 10.1 branch.

        Show
        Deepa Remesh added a comment - Resolved by svn revisions 381937 in trunk (10.2) and 408612 in 10.1 branch.

          People

          • Assignee:
            Deepa Remesh
            Reporter:
            Deepa Remesh
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development