Hi Ron,
hope you are well rested now ;-)
Thanks for the patch and the excellent description.
I run your test cases with sapDB (think supports JDBC 2.0)
without the patched PB and RsIterator, all tests pass well.
Seems that oracle was more severe in handling the resources.
Anyway I add your patch to CVS, works fine with sapDB too.
> ago. In order for this to work properly, I need to utilize a custom
> extension to the StatementManager that's provided with OJB. Since
this
> capability is not 'pluggable' at this time, I need to be able to set
the
> "statementManager" member variable to a new instance of my extension.
This
> member variable is currently scoped as private. I've changed the
scope to
> protected to allow my extension to work. I hope you'll include this
change
> in the code base even though it's not directly related to the bugs
that I
> was trying to correct.
Best solution is to make StatementManager pluggable (using
OJB.properties).
I will try this next days - if this doesn't work, I change the scope of
the
'statementManager' variable in PBImpl.
regards,
Armin
----- Original Message -----
From: "Ron Gallagher" <[EMAIL PROTECTED]>
To: "OJB User Mailing List" <[EMAIL PROTECTED]>
Sent: Tuesday, January 14, 2003 7:05 AM
Subject: [PATCH] RsIterator, PersistenceBrokerImpl and QueryTest
> I've encountered two separate bugs using OJB to accessing an Oracle
> database.
>
> My environment looks like this:
> I'm using the HEAD revision which I downloaded sometime Monday
morning, EST.
> I'm Accessing an Oracle database (8.1.7).
> The JDBC level in my repository.xml is set to 2.0.
>
> For reference, here's the jdbc-descriptor that I'm using:
> <jdbc-connection-descriptor
> jcd-alias="cosmos"
> default-connection="true"
> driver="com.p6spy.engine.spy.P6SpyDriver"
> jdbc-level="2.0"
> platform="Oracle"
> protocol="jdbc"
> subprotocol="oracle"
>
>
dbalias="oci8:@(description=(address_list=(address=(protocol=tcp)(host=b
amba
> m)(port=1521)))(connect_data=(sid=orcl)(server=dedicated)))"
> username="GALLAGHER"
> password="xxx"
> useAutoCommit="1"
> ignoreAutoCommitExceptions="false"
> />
>
> Here's the first problem. I have a table with 2 records in it. If I
submit
> a query with the start index set to 0 and the end index set to 2 or
greater,
> I get a NullPointerException with the following stack trace:
>
> java.lang.NullPointerException
> at
oracle.jdbc.driver.ScrollableResultSet.close(ScrollableResultSet.java)
> at com.p6spy.engine.spy.P6ResultSet.close(P6ResultSet.java:163)
> at
>
o.a.o.b.platforms.PlatformDefaultImpl.beforeStatementClose(PlatformDefau
ltIm
> pl.java:124)
> at
>
o.a.o.b.accesslayer.StatementManager.closeResources(StatementManager.jav
a:14
> 7)
> at
o.a.o.b.accesslayer.RsIterator.releaseDbResources(RsIterator.java:675)
> at
>
o.a.o.b.singlevm.PersistenceBrokerImpl.getCollectionByQuery(PersistenceB
roke
> rImpl.java:1293)
> at
>
o.a.o.b.singlevm.PersistenceBrokerImpl.getCollectionByQuery(PersistenceB
roke
> rImpl.java:1347)
> at
>
o.a.o.b.singlevm.PersistenceBrokerImpl.getCollectionByQuery(PersistenceB
roke
> rImpl.java:1373)
> at
>
o.a.o.b.singlevm.PersistenceBrokerImpl.getCollectionByQuery(PersistenceB
roke
> rImpl.java:1360)
> at
>
o.a.o.b.singlevm.DelegatingPersistenceBroker.getCollectionByQuery(Delega
ting
> PersistenceBroker.java:273)
>
> I added code to RsIterator::releaseDbResources to see who's calling
it, and
> found out that during my test, that method is being called twice. The
first
> time is at line 1246 in PersistenceBrokerImpl::getCollectionByQuery,
and the
> second time is from line 1293 in the same method. Line 1246 is the
"while"
> statement that wraps the loading of the "result" with candidate
records.
> The code that indirectly invokes releaseDbResources is
"iter.hasNext()".
> Line 1293 appears in the "finally" block, and it directly invokes the
> releaseDbResources method.
>
> My dignosis of this problem is that there are two attempts being made
to
> release the resources that are held by the RsIterator that's being
used.
> The first attempt succeeds. The second fails with an NPE. The
solution is
> to change RsIterator so that it (a) keeps track of whether or not the
> resources have been released, and (b) releases resources only if they
have
> not been previously been released. The attached version of
RsIterator.java
> contains this fix. I've also attached an update to QueryTest.java
that
> includes two test cases (testQueryRangeAllRecords and
> testQueryRangeOneMoreThanTotal) that can be used to verify the
presence of
> this problem and the correction of it once RsIterator is updated.
Please
> note that I was only able to manifest this error while connected to
Oracle
> with the jdbc level set to 2.0. The problem did not occur when I ran
the
> OJB unit tests against HSQLDB (jdbc level 1.0 or 2.0) or Oracle (jdbc
level
> 1.0).
>
> Now, that I got past that problem, I immediately ran into another.
This
> problem occurs immediately after the "while" loop in the same method.
> Here's the code:
>
> /**
> * set full size of query to pass back to client.
> */
> if ((startAt == Query.NO_START_AT_INDEX)
> && (endAt == Query.NO_END_AT_INDEX)) {
> query.fullSize(retrievedCount);
> } else {
> query.fullSize(iter.size()); /* This causes the problem */
> }
>
> The call to "iter.size()" causes an NPE becuase it's trying to access
a
> result set that was closed during the last call to "iter.hasNext()".
The
> NPE is thrown if the 'requested record count', as indicated by the
start/end
> indices on the query, is equal to or greater than the number of
records that
> are in the underlying table. The NPE does not occur if the
'requested
> record count' is less than the number of records that are in the
underlying
> table.
>
> There are two parts to the solution to this problem:
>
> Part 1) Change the "while" statement so that "iter.hasNext()" is
evaluated
> after the 'requested record count' is evaluated. Right now, the
"while"
> statement looks like this:
>
> while ( iter.hasNext() and ('records retrieved' < 'requested record
count'))
>
> Since java does short-circuit expression evaluation, as soon as
> "iter.hasNext()" returns false, then the loop is complete. However,
calling
> "iter.hasNext()" has an unintended side-effect. It results in the
closure
> of the associated result set if there are no more records in the
result set.
> When "iter.size()" is called later, the NPE occurs.
>
> If the while statement were changed to the following:
>
> while (('records retrieved' < 'requested record count') and
iter.hasNext())
>
> Then "iter.hasNext()" would not be called if we have retrieved the
requested
> number of records. When we exit the "while" loop, the associated
result set
> is still open and can be used by the "iter.size()" method.
>
> Part 2) Change the logic that's used to set the "fullSize" of the
query in
> the event that a start/end index is specified. Right now, this logic
looks
> like this:
>
> if (<neither start/end index is specified>) {
> query.fullSize(retrievedCount);
> } else {
> query.fullSize(iter.size());
> }
>
> The problem occurs if the user requests more records then are present
in the
> record set. In that case, the "while" loop will exit because
> "iter.hasNext()" returns false. The condition "('records retrieved' <
> 'requested record count')" will never be true. To get around this,
the
> logic that sets the full size of the result set needs to change to the
> following:
>
> if (<neither start/end index is specified>) {
> query.fullSize(retrievedCount);
> } else {
> if (numberOfObjectsToFetch > retrievedCount ) {
> query.fullSize(retrievedCount + query.getStartAtIndex());
> } else {
> query.fullSize(iter.size());
> }
> }
>
> Or, to put it in English:
>
> "If the user asks for more records than are available, set the full
size to
> the number of records that were retrieved plus the number of records
we
> skipped at the beginning of processing"
>
> Once again, I've got test cases for you. If you run the QueryTest
test
> cases after applying the the fix to RsIterator.java, you'll see this
problem
> manifested in the test case "testQueryRangeOneMoreThanTotal". I've
also
> attached an update to QueryTest.java that includes two test cases
> (testQueryRangeAllRecords and testQueryRangeOneMoreThanTotal) that can
be
> used to verify the presence of this problem and the correction of it
once
> RsIterator is updated. Please note that I was only able to manifest
this
> error while connected to Oracle with the jdbc level set to 2.0. The
> problem did not occur when I ran the OJB unit tests against HSQLDB
(jdbc
> level 1.0 or 2.0) or Oracle (jdbc level 1.0).
>
> If you apply the fixes that I've included with this email, all of the
> existing test cases still pass when you run them against Oracle with
the
> jdbc level set to 2.0. An unintended, yet welcome, side affect of
these
> fixes is that two test cases that previously failed when run against
oracle
> at jdbc level 2.0 now pass. These are
testPrefetchedCollectionSingleKey and
> testPrefetchedReferencesSingleKey in QueryTest.
>
> I've included one change to PersistenceBrokerImpl that's not directly
> related to either, but is needed in my current situation. We've got a
> project where we've extended PersistenceBrokerImpl in such a way that
we can
> utilize Oracle packages to do all of our inserts, updates and deletes.
> Thomas and I exchanged various emails related to this a couple of
months
> ago. In order for this to work properly, I need to utilize a custom
> extension to the StatementManager that's provided with OJB. Since
this
> capability is not 'pluggable' at this time, I need to be able to set
the
> "statementManager" member variable to a new instance of my extension.
This
> member variable is currently scoped as private. I've changed the
scope to
> protected to allow my extension to work. I hope you'll include this
change
> in the code base even though it's not directly related to the bugs
that I
> was trying to correct.
>
> I hope I've provided enough detail for you to understand the problems
that I
> found and the fixes that I've provided. I'm tired. I think I'll go
to bed
> now.
>
> Ron Gallagher
> Atlanta, GA
> [EMAIL PROTECTED]
>
------------------------------------------------------------------------
--------
> --
> To unsubscribe, e-mail:
<mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail:
<mailto:[EMAIL PROTECTED]>
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
