Derby
  1. Derby
  2. DERBY-3844

ASSERT failure in BasePage.unlatch() when running LobStreamsTest

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.5.1.1, 10.6.1.0
    • Fix Version/s: 10.5.3.1, 10.6.1.0
    • Component/s: Store
    • Labels:
      None
    • Environment:
      Solaris 10, AMD Opteron with 2 CPUs, Java(TM) SE Runtime Environment (build 1.6.0_06-b02), Derby trunk revision 686755
    • Urgency:
      Normal
    • Issue & fix info:
      Release Note Needed, Repro attached

      Description

      I saw this failure when running suites.All to test the patch nested_transaction_v2.diff which is posted on DERBY-3693. The failure did not occur when I reran the test, and I don't believe the patch should have any effect on the code that failed. The ASSERT that is triggered is this one (which indicates that we're trying to unlatch a page that's not latched):

      if (SanityManager.DEBUG)

      { SanityManager.ASSERT(isLatched()); }

      Here's the full stack trace and thread dump:

      1) testBlobWrite3Param(org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest)org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED
      at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityManager.java:98)
      at org.apache.derby.impl.store.raw.data.BasePage.unlatch(BasePage.java:1319)
      at org.apache.derby.impl.store.raw.data.OverflowInputStream.fillByteHolder(OverflowInputStream.java:152)
      at org.apache.derby.impl.store.raw.data.BufferedByteHolderInputStream.read(BufferedByteHolderInputStream.java:44)
      at java.io.DataInputStream.read(DataInputStream.java:132)
      at org.apache.derby.impl.jdbc.PositionedStoreStream.read(PositionedStoreStream.java:106)
      at org.apache.derby.impl.jdbc.AutoPositioningStream.read(AutoPositioningStream.java:113)
      at org.apache.derby.impl.jdbc.UpdatableBlobStream.read(UpdatableBlobStream.java:194)
      at org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest.readBytesFromStream(LobStreamsTest.java:463)
      at org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest.compareLob2File(LobStreamsTest.java:488)
      at org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest.testBlobWrite3Param(LobStreamsTest.java:130)
      at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:104)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
      at junit.extensions.TestSetup.run(TestSetup.java:25)
      at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
      ---------------
      Stack traces for all live threads:
      Thread name=derby.antiGC id=280 priority=1 state=WAITING isdaemon=true
      java.lang.Object.wait(Native Method)
      java.lang.Object.wait(Object.java:485)
      org.apache.derby.impl.services.monitor.AntiGC.run(BaseMonitor.java:2217)
      java.lang.Thread.run(Thread.java:619)

      Thread name=Signal Dispatcher id=4 priority=9 state=RUNNABLE isdaemon=true

      Thread name=main id=1 priority=5 state=RUNNABLE isdaemon=false
      java.lang.Thread.dumpThreads(Native Method)
      java.lang.Thread.getAllStackTraces(Thread.java:1477)
      org.apache.derby.shared.common.sanity.ThreadDump.getStackDumpString(ThreadDump.java:34)
      sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      org.apache.derby.shared.common.sanity.AssertFailure$1.run(AssertFailure.java:165)
      java.security.AccessController.doPrivileged(Native Method)
      org.apache.derby.shared.common.sanity.AssertFailure.dumpThreads(AssertFailure.java:159)
      org.apache.derby.shared.common.sanity.AssertFailure.<init>(AssertFailure.java:82)
      org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityManager.java:98)
      org.apache.derby.impl.store.raw.data.BasePage.unlatch(BasePage.java:1319)
      org.apache.derby.impl.store.raw.data.OverflowInputStream.fillByteHolder(OverflowInputStream.java:152)
      org.apache.derby.impl.store.raw.data.BufferedByteHolderInputStream.read(BufferedByteHolderInputStream.java:44)
      java.io.DataInputStream.read(DataInputStream.java:132)
      org.apache.derby.impl.jdbc.PositionedStoreStream.read(PositionedStoreStream.java:106)
      org.apache.derby.impl.jdbc.AutoPositioningStream.read(AutoPositioningStream.java:113)
      org.apache.derby.impl.jdbc.UpdatableBlobStream.read(UpdatableBlobStream.java:194)
      org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest.readBytesFromStream(LobStreamsTest.java:463)
      org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest.compareLob2File(LobStreamsTest.java:488)
      org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest.testBlobWrite3Param(LobStreamsTest.java:130)
      sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:104)
      junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
      junit.extensions.TestSetup$1.protect(TestSetup.java:21)
      junit.extensions.TestSetup.run(TestSetup.java:25)
      org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)

      Thread name=derby.rawStoreDaemon id=941 priority=5 state=TIMED_WAITING isdaemon=true
      java.lang.Object.wait(Native Method)
      org.apache.derby.impl.services.daemon.BasicDaemon.rest(BasicDaemon.java:571)
      org.apache.derby.impl.services.daemon.BasicDaemon.run(BasicDaemon.java:388)
      java.lang.Thread.run(Thread.java:619)

      Thread name=Timer-3 id=281 priority=5 state=WAITING isdaemon=true
      java.lang.Object.wait(Native Method)
      java.lang.Object.wait(Object.java:485)
      java.util.TimerThread.mainLoop(Timer.java:483)
      java.util.TimerThread.run(Timer.java:462)

      Thread name=Finalizer id=3 priority=8 state=WAITING isdaemon=true
      java.lang.Object.wait(Native Method)
      java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:116)
      java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:132)
      java.lang.ref.Finalizer$FinalizerThread.run(Finalizer.java:159)

      Thread name=Reference Handler id=2 priority=10 state=WAITING isdaemon=true
      java.lang.Object.wait(Native Method)
      java.lang.Object.wait(Object.java:485)
      java.lang.ref.Reference$ReferenceHandler.run(Reference.java:116)

      Thread name=derby.rawStoreDaemon id=933 priority=5 state=TIMED_WAITING isdaemon=true
      java.lang.Object.wait(Native Method)
      org.apache.derby.impl.services.daemon.BasicDaemon.rest(BasicDaemon.java:571)
      org.apache.derby.impl.services.daemon.BasicDaemon.run(BasicDaemon.java:388)
      java.lang.Thread.run(Thread.java:619)

      ---------------

      1. releaseNote.html
        5 kB
        Kristian Waagan
      2. derby-3844-1c-disallow_multiple_accesses.diff
        48 kB
        Kristian Waagan
      3. releaseNote.html
        5 kB
        Kristian Waagan
      4. derby-3844-2a-jdbcdriver_test_rewrite.diff
        11 kB
        Kristian Waagan
      5. derby-3844-1b-disallow_multiple_accesses.diff
        59 kB
        Kristian Waagan
      6. derby-3844-1a-disallow_multiple_accesses.stat
        2 kB
        Kristian Waagan
      7. derby-3844-1a-disallow_multiple_accesses.diff
        58 kB
        Kristian Waagan
      8. derby-3844-1a-repro_test.diff
        3 kB
        Kristian Waagan

        Issue Links

          Activity

          Hide
          Kristian Waagan added a comment -

          Triaged July 3, 2009: Assigned normal urgency.

          Show
          Kristian Waagan added a comment - Triaged July 3, 2009: Assigned normal urgency.
          Hide
          Kristian Waagan added a comment -

          Attaching a repro for the bug.
          Even though generating the junk data seems to trigger garbage collection, I added calls to do gc and finalization for good measure (I'm aware that the call to do gc may be a no-op).

          The trouble in this case is the use of the pattern rs.getBlob(1).blobMethod(), followed by another operation on the same BLOB field, and combined with gc/finalization. The result set generates a new Blob object for each call to getBlob, and this object has a finalize-method closing the source stream (and unlatching the source page).

          Without having studied this in detail, I think there are two main lines of fixing this:
          1) Disallow calling getBlob multiple times on the same column (and the same row).
          2) Make the result set smarter, or somehow make sure the source stream isn't closed too early.

          (1) is the simplest, but may break existing applications (including some of our own tests for sure). I have not consulted the JDBC spec on this.
          (2) is probably harder, may have performance impacts and we run the risk of introducing resource leaks (this may be cleaned up in commit/rollback anyway). I think it should be possible to get this right, but it will make the result set slightly fatter.

          The discussion about calling various result set methods multiple times seems familiar, but I'm not sure we reached consensus the last time...
          The Java SE 6 JavaDoc isn't very authoritative:
          "For maximum portability, result set columns within each row should be read in left-to-right order, and each column should be read only once."

          Having gotten this far in my post, won't this be a problem anyway if the user calls Blob.free?
          Assuming obtaining multiple Blob-objects for the same BLOB field is allowed, should a Blob.free affect the other "instances"?

          I fear we are about to wander off into the tall weeds, and in my opinion we should try to improve the situation such that it is clear to the user what is allowed and what isn't when it comes to LOBs and streams. I don't know how much stricter we would have to be, and I'm willing to postpone this until 11.0, but I'd rather do it earlier.
          I'm far from having the overview, but note that other situations get us into trouble too. We already have Jiras logged where queries like "select blob as b1, blob as b2" cause errors.

          Show
          Kristian Waagan added a comment - Attaching a repro for the bug. Even though generating the junk data seems to trigger garbage collection, I added calls to do gc and finalization for good measure (I'm aware that the call to do gc may be a no-op). The trouble in this case is the use of the pattern rs.getBlob(1).blobMethod(), followed by another operation on the same BLOB field, and combined with gc/finalization. The result set generates a new Blob object for each call to getBlob, and this object has a finalize-method closing the source stream (and unlatching the source page). Without having studied this in detail, I think there are two main lines of fixing this: 1) Disallow calling getBlob multiple times on the same column (and the same row). 2) Make the result set smarter, or somehow make sure the source stream isn't closed too early. (1) is the simplest, but may break existing applications (including some of our own tests for sure). I have not consulted the JDBC spec on this. (2) is probably harder, may have performance impacts and we run the risk of introducing resource leaks (this may be cleaned up in commit/rollback anyway). I think it should be possible to get this right, but it will make the result set slightly fatter. The discussion about calling various result set methods multiple times seems familiar, but I'm not sure we reached consensus the last time... The Java SE 6 JavaDoc isn't very authoritative: "For maximum portability, result set columns within each row should be read in left-to-right order, and each column should be read only once." Having gotten this far in my post, won't this be a problem anyway if the user calls Blob.free? Assuming obtaining multiple Blob-objects for the same BLOB field is allowed, should a Blob.free affect the other "instances"? I fear we are about to wander off into the tall weeds, and in my opinion we should try to improve the situation such that it is clear to the user what is allowed and what isn't when it comes to LOBs and streams. I don't know how much stricter we would have to be, and I'm willing to postpone this until 11.0, but I'd rather do it earlier. I'm far from having the overview, but note that other situations get us into trouble too. We already have Jiras logged where queries like "select blob as b1, blob as b2" cause errors.
          Hide
          Bryan Pendleton added a comment -

          The two separate calls to getBlob() share the same underlying stream, and the finalizer for the first blob
          object closes the stream out from underneath the second blob? Is that correct?

          Could we use a reference-counted wrapper around the stream so that each blob could record
          its own "interest" in the blob, and would know when it was the last blob to release the stream,
          and thus it was safe to close it?

          Show
          Bryan Pendleton added a comment - The two separate calls to getBlob() share the same underlying stream, and the finalizer for the first blob object closes the stream out from underneath the second blob? Is that correct? Could we use a reference-counted wrapper around the stream so that each blob could record its own "interest" in the blob, and would know when it was the last blob to release the stream, and thus it was safe to close it?
          Hide
          Kristian Waagan added a comment -

          Your understanding is correct, Bryan.

          Something along the lines you describe will most probably work, but may require changes to the stream class down in store. You have one real source stream, but other streams may be "spliced off" from it. Add another couple of layers and you're up into the outer JDBC realm

          Unfortunately, I think we have issues we cannot address at this level. I don't have a deep enough understanding of what's going on yet, but I hope to spend some more time investigating soon.
          One thing we have to do, is to agree on what is going to be allowed. From the realm of Blobs, consider these pieces of pseudo-code:

          1) select blob as b1, blob as b2 from blobtable

          2) // Pattern where the Blob references aren't kept. GC has side-effects currently.
          stream = rs.getBlob(1).getBinaryStream()
          length = rs.getBlob(1).length()
          // Do something here, assume GC / finalization will take place.
          // Now read from the stream.
          stream.read(buf) // <-- fails

          3) Blob b1 = rs.getBlob(1)
          // Process b1.
          // Note sure if b2 will be invalid if b1.free() is called here, or if obtaining b2 will latch/lock the resources again.
          // If you obtain b1 and b2 at the same time, and then call free on one of them, the other will be invalidated.
          Blob b2 = rs.getBlob(1)
          // Process b2.

          4) Blob b1 = rs.getBlob(1)
          stream1 = b1.getBinaryStream(1024, 4096)
          stream2 = b1.getBinaryStream(10240, 2048)
          // Process both streams "concurrently"

          • (1) is definitely allowed by SQL. This is the simple example, a cross join is more complex and may end up referencing the same Blob multiple times (in different rows of the result).
          • In (2), should it be forbidden to call getBlob(1) multiple times, should we return the same Blob object, or should the two Blobs be independent on each other? This gets worse if updates come into play...
          • Case (3) is much like (2), but the effects on the two Blob objects are caused by explicit method calls only.
          • (4) adds yet another level, and here we support update-aware streams currently. In this case I believe we're talking about changes to this specific Blob-instance.

          I think (1) and (4) may be the more natural use-cases, whereas (2) and (3) can be categorized as corner cases . All issues here are related to having several handles to the same source data.
          With the exception of class (1), the user can work around these issues by changing the application code (unless a framework generates / uses code that trigger these issues). The whole thing would just be simpler to deal with if there were clear rules

          Related Jiras: DERBY-3645, DERBY-3646 and DERBY-3650 (there are probably more)

          Show
          Kristian Waagan added a comment - Your understanding is correct, Bryan. Something along the lines you describe will most probably work, but may require changes to the stream class down in store. You have one real source stream, but other streams may be "spliced off" from it. Add another couple of layers and you're up into the outer JDBC realm Unfortunately, I think we have issues we cannot address at this level. I don't have a deep enough understanding of what's going on yet, but I hope to spend some more time investigating soon. One thing we have to do, is to agree on what is going to be allowed. From the realm of Blobs, consider these pieces of pseudo-code: 1) select blob as b1, blob as b2 from blobtable 2) // Pattern where the Blob references aren't kept. GC has side-effects currently. stream = rs.getBlob(1).getBinaryStream() length = rs.getBlob(1).length() // Do something here, assume GC / finalization will take place. // Now read from the stream. stream.read(buf) // <-- fails 3) Blob b1 = rs.getBlob(1) // Process b1. // Note sure if b2 will be invalid if b1.free() is called here, or if obtaining b2 will latch/lock the resources again. // If you obtain b1 and b2 at the same time, and then call free on one of them, the other will be invalidated. Blob b2 = rs.getBlob(1) // Process b2. 4) Blob b1 = rs.getBlob(1) stream1 = b1.getBinaryStream(1024, 4096) stream2 = b1.getBinaryStream(10240, 2048) // Process both streams "concurrently" (1) is definitely allowed by SQL. This is the simple example, a cross join is more complex and may end up referencing the same Blob multiple times (in different rows of the result). In (2), should it be forbidden to call getBlob(1) multiple times, should we return the same Blob object, or should the two Blobs be independent on each other? This gets worse if updates come into play... Case (3) is much like (2), but the effects on the two Blob objects are caused by explicit method calls only. (4) adds yet another level, and here we support update-aware streams currently. In this case I believe we're talking about changes to this specific Blob-instance. I think (1) and (4) may be the more natural use-cases, whereas (2) and (3) can be categorized as corner cases . All issues here are related to having several handles to the same source data. With the exception of class (1), the user can work around these issues by changing the application code (unless a framework generates / uses code that trigger these issues). The whole thing would just be simpler to deal with if there were clear rules Related Jiras: DERBY-3645 , DERBY-3646 and DERBY-3650 (there are probably more)
          Hide
          Kristian Waagan added a comment -

          We should address this issue, as it is rather confusing for the user and it might also be hard to debug (timing dependent).

          I propose tightening the rules, such that it is only allowed to access a LOB column once (per row) with the ResultSet.get-methods. This will be very much like what we do for the getXStream-methods already.
          The JDBC standard doesn't say much about this, but this sentence from the Java SE 6 API JavaDoc does apply:
          "For maximum portability, result set columns within each row should be read in left-to-right order, and each column should be read only once."
          In the list above this means that usages (2) and (3) will be disallowed.

          Implementing this restriction includes at least the following work:

          • add code to track column access (for LOB-columns) in the result set implementations
          • write a release note, as this change may break existing applications
          • rewrite a few of our tests (got 41 errors in total for suites.All, but some of these were caused by resources not being cleaned up after the first failure caused by the change)
            For instance: testClobCharacterWrite1Char(org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest)java.sql.SQLException: Complex column value (stream or LOB) in result cannot be retrieved twice
            In this case I just reused the message for streams and modified it to fit both. Should there be two different messages or can the example text above be improved?

          As an experiment I also tried using the cloning methods added in DERBY-4520. This seemed to work, the repro passed. This suggests that we can add support for retrieving a LOB twice at a later time if we get such requests.
          However, I haven't investigated fully, and I don't know if there are situations where we have to serve data from non-store streams that aren't implementing CloneableStream.

          What do people think about the suggested restriction?

          Show
          Kristian Waagan added a comment - We should address this issue, as it is rather confusing for the user and it might also be hard to debug (timing dependent). I propose tightening the rules, such that it is only allowed to access a LOB column once (per row) with the ResultSet.get-methods. This will be very much like what we do for the getXStream-methods already. The JDBC standard doesn't say much about this, but this sentence from the Java SE 6 API JavaDoc does apply: "For maximum portability, result set columns within each row should be read in left-to-right order, and each column should be read only once." In the list above this means that usages (2) and (3) will be disallowed. Implementing this restriction includes at least the following work: add code to track column access (for LOB-columns) in the result set implementations write a release note, as this change may break existing applications rewrite a few of our tests (got 41 errors in total for suites.All, but some of these were caused by resources not being cleaned up after the first failure caused by the change) For instance: testClobCharacterWrite1Char(org.apache.derbyTesting.functionTests.tests.jdbcapi.LobStreamsTest)java.sql.SQLException: Complex column value (stream or LOB) in result cannot be retrieved twice In this case I just reused the message for streams and modified it to fit both. Should there be two different messages or can the example text above be improved? As an experiment I also tried using the cloning methods added in DERBY-4520 . This seemed to work, the repro passed. This suggests that we can add support for retrieving a LOB twice at a later time if we get such requests. However, I haven't investigated fully, and I don't know if there are situations where we have to serve data from non-store streams that aren't implementing CloneableStream. What do people think about the suggested restriction?
          Hide
          Knut Anders Hatlen added a comment -

          I think it sounds like a reasonable restriction to only allow LOB values to be accessed once. Failing gracefully sounds like a much better option than the current behaviour with intermittent assert failures and Java-level deadlocks. Of course, it would be nice to have a more liberal implementation, but that could be added later if someone finds it useful and feels like implementing it.

          Show
          Knut Anders Hatlen added a comment - I think it sounds like a reasonable restriction to only allow LOB values to be accessed once. Failing gracefully sounds like a much better option than the current behaviour with intermittent assert failures and Java-level deadlocks. Of course, it would be nice to have a more liberal implementation, but that could be added later if someone finds it useful and feels like implementing it.
          Hide
          Knut Anders Hatlen added a comment -

          > java.sql.SQLException: Complex column value (stream or LOB) in
          > result cannot be retrieved twice
          >
          > In this case I just reused the message for streams and modified it
          > to fit both. Should there be two different messages or can the
          > example text above be improved?

          I think it's fine to share the message between these two cases, since
          they are so similar. I'd remove "complex column value", I think, and
          just say

          Stream or LOB value cannot be retrieved more than once

          Show
          Knut Anders Hatlen added a comment - > java.sql.SQLException: Complex column value (stream or LOB) in > result cannot be retrieved twice > > In this case I just reused the message for streams and modified it > to fit both. Should there be two different messages or can the > example text above be improved? I think it's fine to share the message between these two cases, since they are so similar. I'd remove "complex column value", I think, and just say Stream or LOB value cannot be retrieved more than once
          Hide
          Kristian Waagan added a comment -

          Patch 1a disallows accessing a LOB column more than once.
          High level patch description:
          o modified message XCL18, and deleted all the translations of it
          o added DataValueDescriptor.hasStream, which makes it clearer where we actually want to fetch the stream and where we just want to check if there is a stream or not

          • currently only SQLBlob and SQLClob override the default implementation
            o replaced several dvd.getStream() != null with dvd.hashStream()
          • as part of this I also refactored some code in EmbedBlob
            o renamed variables and methods for checking if a column has already been accessed
            o added checks if the LOB column has already been accessed in getBlob and getBlob, both in the embedded and the client driver
          • also added some more comments
          • made the boolean array (columnUsedFlags_) be lazily initialized in the client driver (this included basically rewriting two small methods)
            o modified a few tests failing because of the new restriction
            o added two regression tests in LobStreamsTest

          I will also have to write a release note for this fix, as existing applications may be affected by it.

          I'm running the full regression tests now, will post the result when they're done.
          Patch ready for review.

          Show
          Kristian Waagan added a comment - Patch 1a disallows accessing a LOB column more than once. High level patch description: o modified message XCL18, and deleted all the translations of it o added DataValueDescriptor.hasStream, which makes it clearer where we actually want to fetch the stream and where we just want to check if there is a stream or not currently only SQLBlob and SQLClob override the default implementation o replaced several dvd.getStream() != null with dvd.hashStream() as part of this I also refactored some code in EmbedBlob o renamed variables and methods for checking if a column has already been accessed o added checks if the LOB column has already been accessed in getBlob and getBlob, both in the embedded and the client driver also added some more comments made the boolean array (columnUsedFlags_) be lazily initialized in the client driver (this included basically rewriting two small methods) o modified a few tests failing because of the new restriction o added two regression tests in LobStreamsTest I will also have to write a release note for this fix, as existing applications may be affected by it. I'm running the full regression tests now, will post the result when they're done. Patch ready for review.
          Hide
          Kristian Waagan added a comment -

          Patch 1b fixes errors in jdbcapi.ClosedObjectTest discovered when running the full regression tests by moving the call to useStreamOrLOB to after the check for if the result set is open (failed with NPE because the meta data is cleared when closing).

          Patch ready for review, re-running tests.

          Show
          Kristian Waagan added a comment - Patch 1b fixes errors in jdbcapi.ClosedObjectTest discovered when running the full regression tests by moving the call to useStreamOrLOB to after the check for if the result set is open (failed with NPE because the meta data is cleared when closing). Patch ready for review, re-running tests.
          Hide
          Kristian Waagan added a comment -

          I overlooked an error in the compatibility suite. Again, the JDBCDriverTest fails because it fetched LOBs several times from the same column on a single row.
          Patch 2a rewrites the test, basically be iterating over the different types and coercions instead of iterating over the rows. The query is executed once per check.
          Note that I deleted some unused methods in JDBCDriverTest. There were no comments indicating they would be useful for debugging, so I assumed they were left-overs from the test development phase.

          I compared the output from the original test and the new test to make sure that no tests were lost during the rewrite (to see the output you have to run with drb.tests.debug=true).

          Patch 1b depends on 2a to run cleanly.
          Patch ready for review.

          Show
          Kristian Waagan added a comment - I overlooked an error in the compatibility suite. Again, the JDBCDriverTest fails because it fetched LOBs several times from the same column on a single row. Patch 2a rewrites the test, basically be iterating over the different types and coercions instead of iterating over the rows. The query is executed once per check. Note that I deleted some unused methods in JDBCDriverTest. There were no comments indicating they would be useful for debugging, so I assumed they were left-overs from the test development phase. I compared the output from the original test and the new test to make sure that no tests were lost during the rewrite (to see the output you have to run with drb.tests.debug=true). Patch 1b depends on 2a to run cleanly. Patch ready for review.
          Hide
          Kristian Waagan added a comment -

          First take on a release note.

          Show
          Kristian Waagan added a comment - First take on a release note.
          Hide
          Kristian Waagan added a comment -

          Committed patch 2a to trunk with revision 910481.
          I'll await at least one successful test run in the nightly regression test before considering committing patch 1a.

          Show
          Kristian Waagan added a comment - Committed patch 2a to trunk with revision 910481. I'll await at least one successful test run in the nightly regression test before considering committing patch 1a.
          Hide
          Kristian Waagan added a comment -

          Attached patch 1c.
          I pulled out the cleanup code, as it just cluttered up the patch too much (will address the issue in a separate Jira).

          Committed patch 1c to trunk with revision 911793.

          I think this concludes the code work on this issue. Feel free to review the release note.

          Show
          Kristian Waagan added a comment - Attached patch 1c. I pulled out the cleanup code, as it just cluttered up the patch too much (will address the issue in a separate Jira). Committed patch 1c to trunk with revision 911793. I think this concludes the code work on this issue. Feel free to review the release note.
          Hide
          Knut Anders Hatlen added a comment -

          The release note looks good to me. (One small typo, though: Applications which obtains -> Applications which obtain)

          Show
          Knut Anders Hatlen added a comment - The release note looks good to me. (One small typo, though: Applications which obtains -> Applications which obtain)
          Hide
          Kristian Waagan added a comment -

          Thank you, Knut Anders.

          I fixed the typo and uploaded the release note again.

          Show
          Kristian Waagan added a comment - Thank you, Knut Anders. I fixed the typo and uploaded the release note again.
          Hide
          Kristian Waagan added a comment -

          Resolving issue.

          This fix could be back-ported to 10.5 (maybe even 10.4), but that may break existing applications using get[BC]lob. The problem can / must be fixed in the application code. No changes to the Derby source code is required, but without the changes in patch 1a it is up to the application programmer to do the right thing (Derby won't scream).

          I'm not sure how likely it is that existing applications calling get[BC]lob multiple times work, but since the bug is timing dependent there is a slight chance.
          One argument for back-porting the fix is that one of the symptoms of the underlying problem is a Java deadlock (the other is an exception being raised).

          Please advice.

          Show
          Kristian Waagan added a comment - Resolving issue. This fix could be back-ported to 10.5 (maybe even 10.4), but that may break existing applications using get [BC] lob. The problem can / must be fixed in the application code. No changes to the Derby source code is required, but without the changes in patch 1a it is up to the application programmer to do the right thing (Derby won't scream). I'm not sure how likely it is that existing applications calling get [BC] lob multiple times work, but since the bug is timing dependent there is a slight chance. One argument for back-porting the fix is that one of the symptoms of the underlying problem is a Java deadlock (the other is an exception being raised). Please advice.
          Hide
          Knut Anders Hatlen added a comment -

          I think it's fine not to back-port the fix, since there's a known workaround if someone experiences the bug.

          Show
          Knut Anders Hatlen added a comment - I think it's fine not to back-port the fix, since there's a known workaround if someone experiences the bug.
          Hide
          Lily Wei added a comment -

          Reopen to 10.5 back port

          Show
          Lily Wei added a comment - Reopen to 10.5 back port
          Hide
          Kathey Marsden added a comment -

          Assigned to myself for 10.5 backport

          Show
          Kathey Marsden added a comment - Assigned to myself for 10.5 backport
          Hide
          Kathey Marsden added a comment -

          completed 10.5 back port

          Show
          Kathey Marsden added a comment - completed 10.5 back port

            People

            • Assignee:
              Kristian Waagan
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development