You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Deepa Remesh (JIRA)" <de...@db.apache.org> on 2006/02/17 18:05:27 UTC
[jira] Created: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
---------------------------------------------------------------------------------
Key: DERBY-1002
URL: http://issues.apache.org/jira/browse/DERBY-1002
Project: Derby
Type: Improvement
Components: Network Server
Reporter: Deepa Remesh
Priority: Trivial
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.
--
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
Re: [jira] Updated: (DERBY-1002) Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
Posted by Deepa Remesh <dr...@gmail.com>.
Thanks Bryan for reading my patch. Please see my answers below:
On 2/26/06, Bryan Pendleton <bp...@amberpoint.com> wrote:
> Three small comments:
>
> 1) It wasn't clear to me why these needed to be applied as two
> separate patches. I would be comfortable putting the test and the
> fix in the same commit; is there something I'm missing here?
The patches can be committed together. It is just that patch2 (tests)
should not be committed before patch1 (code changes). I'll mention
this when I upload the next version of patch.
>
> 2) There's a mysterious comment in DRDAStatement.initialize():
>
> + * TODO: Need to see what exactly it means to initialize the default
> + * statement. (DERBY-1002)
>
> Does this mean that there is still a loose end to tie up here?
>
Yes. The comments in the code where DRDAStatement.initialize() is
being called is confusing. They say "// initialize statement for
reuse". I think this is confusing because we have reset() method which
also is called to re-use statements stored in stmtTable. initialize()
is called for default statement, which is used differently. From what
I understand, there is just one default statement per database and
default statement is different in the way it gets initialized and
used. e.g: stmt variable used in default statement is created only
once in Database.makeConnection. So, we cannot call reset() for
default statement. I think it would be good to understand this part
and fix up the comments/code so that it is clear. Kathey and Knut also
mentioned that the use of initialize method at few places was
confusing to them. Sorry, I did not mention explicitly that the patch
has more to-do items in my latest comment.
> 3) It took me a little while to realize that the idea of testStatementReuse
> is that it sets up the section tables in such a way as to provoke statement
> re-use in tests jira_491_492 and testImplicitClose.
[snip ... very good suggestions to improve comments in the test case]
Thanks for the suggestions. I'll fix the comments and upload a new
version of patch2.
Thanks,
Deepa
Re: [jira] Updated: (DERBY-1002) Check that DRDAStatement and DRDAResultSet
states are reset when they are re-used
Posted by Bryan Pendleton <bp...@amberpoint.com>.
Hi Deepa,
Thanks for pulling this patch together. This is excellent work,
and I think the new structure of initialize/close/reset/setTypDefValues
is much cleaner and easier to read.
I successfully applied your patches, and I can confirm that your
new regression test provokes the expected failure(s) without the
patch, and succeeds with the patch.
Three small comments:
1) It wasn't clear to me why these needed to be applied as two
separate patches. I would be comfortable putting the test and the
fix in the same commit; is there something I'm missing here?
2) There's a mysterious comment in DRDAStatement.initialize():
+ * TODO: Need to see what exactly it means to initialize the default
+ * statement. (DERBY-1002)
Does this mean that there is still a loose end to tie up here?
3) It took me a little while to realize that the idea of testStatementReuse
is that it sets up the section tables in such a way as to provoke statement
re-use in tests jira_491_492 and testImplicitClose. That is, testStatementReuse
isn't directly testing the problem, but rather is setting things up so that
the other two tests will see a "more interesting" environment.
It seems like it would be nice to be more explicit about this technique
in the comments at least, because it's kind of subtle and when I first read
the new test code, I thought that you had omitted something because the
end of testStatementReuse() seems to trail off without testing anything;
moreover, it refers to something called cSt2 which doesn't actually exist in
that test case:
+ // Close cSt1 so that cSt2 will reuse same DRDAStatement
+ cSt1.close();
My suggestion is:
- fix that last comment in testStatementReuse to make it clear that we are
arranging to have a re-usable statement be "ready" at the end of this
method
- fix the method-level comments for testStatementReuse so that instead of
saying:
+ * This test creates statements and closes them to provoke the network server to
+ * re-use statements to test that re-use happens correctly. It gives error without
+ * patch for DERBY-1002.
they instead make it clear that this test sets up a statement which is ready to be
re-used, so that the *next* test will start by reusing an old statement.
- add a one-line comment at the two places (in testImplictClose() and in
jira_491_492()) where you have planted the call to testStatementReuse() saying
something like:
// Cause an existing statement to be opened and closed so that it will be re-used
Hope this is helpful,
bryan
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Attachment: derby1002-patch1-v1.diff
derby1002-patch1-v1.status
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.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Attachment: derby1002-patch2-v1.diff
derby1002-patch2-v1.status
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
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v1.diff, derby1002-patch2-v1.status, derby1002.java
>
> 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.
--
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
Re: [jira] Updated: (DERBY-1002) Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
Posted by Deepa Remesh <dr...@gmail.com>.
On 2/28/06, Bryan Pendleton <bp...@amberpoint.com> wrote:
> Deepa Remesh (JIRA) wrote:
> > Attaching 'derby1002-patch2-v2.diff' for Bryans' comments
>
> Thanks Deepa. The comments in procedure.java are quite helpful and
> the behavior of the new test code is much clearer now.
>
Thanks for reading the new patch.
If there are no further comments on patches 1 and 2, I would
appreciate if someone can commit these patches since they are blocking
DERBY-210.
Please see the description of patches at
http://issues.apache.org/jira/browse/DERBY-1002#action_12368008
Thanks,
Deepa
Re: [jira] Updated: (DERBY-1002) Check that DRDAStatement and DRDAResultSet
states are reset when they are re-used
Posted by Bryan Pendleton <bp...@amberpoint.com>.
Deepa Remesh (JIRA) wrote:
> Attaching 'derby1002-patch2-v2.diff' for Bryans' comments
Thanks Deepa. The comments in procedure.java are quite helpful and
the behavior of the new test code is much clearer now.
bryan
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Attachment: derby1002-patch2-v2.diff
derby1002-patch2-v2.status
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.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Commented: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=comments#action_12415175 ]
Deepa Remesh commented on DERBY-1002:
-------------------------------------
Just after posting this, I found I can delete links by using "Manage Links". I have removed the links.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Fix Version: 10.1.3.0
Patch 1 and 2 have been committed to trunk as well as 10.1 branch.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Fix For: 10.2.0.0, 10.1.3.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Other Info: [Patch available]
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v1.diff, derby1002-patch2-v1.status, derby1002.java
>
> 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.
--
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
[jira] Commented: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Andrew McIntyre (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=comments#action_12415957 ]
Andrew McIntyre commented on DERBY-1002:
----------------------------------------
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.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Fix For: 10.2.0.0, 10.1.3.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Commented: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=comments#action_12416016 ]
Deepa Remesh commented on DERBY-1002:
-------------------------------------
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.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Fix For: 10.2.0.0, 10.1.3.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Attachment: derby1002-patch1-draft1.diff
derby1002-patch1-draft1.status
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.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Attachment: (was: derby1002-patch2-v1.diff)
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002.java
>
> 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.
--
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
[jira] Commented: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Kathey Marsden (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=comments#action_12416012 ]
Kathey Marsden commented on DERBY-1002:
---------------------------------------
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?
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Fix For: 10.2.0.0, 10.1.3.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Closed: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh closed DERBY-1002:
-------------------------------
Resolution: Fixed
Assign To: Deepa Remesh
Resolved by svn revisions 381937 in trunk (10.2) and 408612 in 10.1 branch.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0, 10.1.3.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Andrew McIntyre (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Andrew McIntyre updated DERBY-1002:
-----------------------------------
Other Info: (was: [Patch available])
Derby Info: [Patch Available]
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Derby Info: (was: [Patch Available])
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.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Attachment: (was: derby1002-patch2-v1.status)
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Assign To: (was: Deepa Remesh)
There are two minor todo items for this issue. I am not working on them currently, hence unassigning myself.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002-patch1-draft1.diff, derby1002-patch1-draft1.status, derby1002-patch1-v1.diff, derby1002-patch1-v1.status, derby1002-patch2-v2.diff, derby1002-patch2-v2.status, derby1002.java
>
> 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.
--
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
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
Attachment: derby1002.java
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.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
> Attachments: derby1002.java
>
> 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.
--
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
Re: [jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by Knut Anders Hatlen <Kn...@Sun.COM>.
"Deepa Remesh (JIRA)" <de...@db.apache.org> writes:
> Deepa Remesh updated DERBY-1002:
> --------------------------------
>
> type: Bug (was: Improvement)
> Fix Version: 10.2.0.0
> Assign To: Deepa Remesh
> Priority: Major (was: Trivial)
>
> 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.
You're right. qryclsimp should be reset in close(). EXCSQLSTT should
never close queries implicitly and OPNQRY will modify the value of
qryclsimp when setOPNQRYOptions() is called, so Codepoint.QRYCLSIMP_NO
is the correct value.
Would it be a good idea to have a method for initializing variables
that could be called both from the constructor and from close()? It
seems like there are a lot of variables that are not reset in
close(). Most of them are probably reset somewhere else, but the lack
of encapsulation makes it very hard to determine which are reset
correctly and which are not.
--
Knut Anders
[jira] Updated: (DERBY-1002) Check that DRDAStatement and
DRDAResultSet states are reset when they are re-used
Posted by "Deepa Remesh (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-1002?page=all ]
Deepa Remesh updated DERBY-1002:
--------------------------------
type: Bug (was: Improvement)
Fix Version: 10.2.0.0
Assign To: Deepa Remesh
Priority: Major (was: Trivial)
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.
> Check that DRDAStatement and DRDAResultSet states are reset when they are re-used
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1002
> URL: http://issues.apache.org/jira/browse/DERBY-1002
> Project: Derby
> Type: Bug
> Components: Network Server
> Reporter: Deepa Remesh
> Assignee: Deepa Remesh
> Fix For: 10.2.0.0
>
> 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.
--
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