Derby
  1. Derby
  2. DERBY-3354

Select from large lob table with embedded gives OutOfMemoryError

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.3.1.4, 10.3.2.1, 10.4.1.3
    • Fix Version/s: 10.3.3.0, 10.4.1.3, 10.5.1.1
    • Component/s: SQL
    • Labels:
      None
    • Bug behavior facts:
      Regression

      Description

      Retrieving from a large table with lobs gives an OutOfMemoryException, even if free() is explictly called on the lob. I believe this is because EmbedConnection.addLobMapping is called for every lob creation but is never cleared until commit or rollback, even if the lob is freed.

      1. LocLeak.java
        1 kB
        Kathey Marsden
      2. derby-3354.diff
        0.9 kB
        Øystein Grøvlen
      3. derby-3354_preview.diff
        3 kB
        Anurag Shekhar
      4. derby-3354_v1.diff
        5 kB
        Anurag Shekhar
      5. derby-3354_v2.diff
        5 kB
        Anurag Shekhar
      6. derby-3354_v3.diff
        5 kB
        Anurag Shekhar
      7. derby-3354_v4.diff
        5 kB
        Anurag Shekhar

        Activity

        Hide
        Kathey Marsden added a comment -

        Atttached is a program to reproduce this issue.
        Run java -Xmx16M LocLeak. I got the error on the 68000 row of retreival with the free(). Sooner if I didn't call free.

        Exception in thread "main" java.lang.OutOfMemoryError: Java heap space
        at java.util.Arrays.copyOf(Arrays.java:2882)
        at java.lang.AbstractStringBuilder.expandCapacity(AbstractStringBuilder.java:100)
        at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:390)
        at java.lang.StringBuilder.append(StringBuilder.java:119)
        at java.lang.Object.toString(Object.java:219)
        at java.lang.String.valueOf(String.java:2827)
        at java.lang.StringBuffer.append(StringBuffer.java:219)
        at org.apache.derby.impl.jdbc.EmbedConnection.restoreContextStack(EmbedConnection.java:1960)
        at org.apache.derby.impl.jdbc.ConnectionChild.restoreContextStack(ConnectionChild.java:131)
        at org.apache.derby.impl.jdbc.UTF8Reader.fillBuffer(UTF8Reader.java:567)
        at org.apache.derby.impl.jdbc.UTF8Reader.read(UTF8Reader.java:245)
        at org.apache.derby.impl.jdbc.ClobUpdatableReader.read(ClobUpdatableReader.java:154)
        at org.apache.derby.impl.jdbc.EmbedClob.getSubString(EmbedClob.java:225)
        at LocLeak.dump_db(LocLeak.java:50)
        at LocLeak.main(LocLeak.java:13)

        Note there may be problems freeing the locators on garbage collection, because client relies upon them even after there is no embedded reference.

        Show
        Kathey Marsden added a comment - Atttached is a program to reproduce this issue. Run java -Xmx16M LocLeak. I got the error on the 68000 row of retreival with the free(). Sooner if I didn't call free. Exception in thread "main" java.lang.OutOfMemoryError: Java heap space at java.util.Arrays.copyOf(Arrays.java:2882) at java.lang.AbstractStringBuilder.expandCapacity(AbstractStringBuilder.java:100) at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:390) at java.lang.StringBuilder.append(StringBuilder.java:119) at java.lang.Object.toString(Object.java:219) at java.lang.String.valueOf(String.java:2827) at java.lang.StringBuffer.append(StringBuffer.java:219) at org.apache.derby.impl.jdbc.EmbedConnection.restoreContextStack(EmbedConnection.java:1960) at org.apache.derby.impl.jdbc.ConnectionChild.restoreContextStack(ConnectionChild.java:131) at org.apache.derby.impl.jdbc.UTF8Reader.fillBuffer(UTF8Reader.java:567) at org.apache.derby.impl.jdbc.UTF8Reader.read(UTF8Reader.java:245) at org.apache.derby.impl.jdbc.ClobUpdatableReader.read(ClobUpdatableReader.java:154) at org.apache.derby.impl.jdbc.EmbedClob.getSubString(EmbedClob.java:225) at LocLeak.dump_db(LocLeak.java:50) at LocLeak.main(LocLeak.java:13) Note there may be problems freeing the locators on garbage collection, because client relies upon them even after there is no embedded reference.
        Hide
        Kathey Marsden added a comment -

        Marking as regression as I confirmed this does not occur with 10.2.2.0

        Show
        Kathey Marsden added a comment - Marking as regression as I confirmed this does not occur with 10.2.2.0
        Hide
        Kathey Marsden added a comment -

        This regression was caused by the following checkin:
        r546881 | bernt | 2007-06-13 07:06:30 -0700 (Wed, 13 Jun 2007) | 1 line

        DERBY-2787 make entry for clob in connection so that temporary file is removed when a connectionn is commited/rolledback. Submitted by Anurag Shekhar

        Show
        Kathey Marsden added a comment - This regression was caused by the following checkin: r546881 | bernt | 2007-06-13 07:06:30 -0700 (Wed, 13 Jun 2007) | 1 line DERBY-2787 make entry for clob in connection so that temporary file is removed when a connectionn is commited/rolledback. Submitted by Anurag Shekhar
        Hide
        Knut Anders Hatlen added a comment -

        > I believe this is because EmbedConnection.addLobMapping is called for every lob creation but is never cleared until commit or rollback, even if the lob is freed.

        I believe your analysis is correct. free() should remove the mapping. Unfortunately, the lob doesn't know about its locator value, but it seems like your about to change that by adding a getLocator() method in DERBY-3243.

        On DERBY-3243 I wrote:
        > Another thing I came to think about: Would it be better to remove addLOBMapping() from the constructors in EmbedBlob/EmbedClob and instead call it lazily from getLocator()? (...) Then we'd also remove the overhead of maintaining the lob mapping in embedded mode.

        I think that change would fix the OOME (on embedded, but we'd probably still see it in a client/server environment).

        Now, it seems like DERBY-2787 added the call to addLOBMapping() intentionally to ensure that free() was called on commit/rollback and temporary files would be deleted. Moving addLOBMapping() out of the constructors would probably reintroduce the problem with temporary files not being deleted. However, the lobs only create temporary files when they are modified and the new size of the lob is greater than the buffer in LOBStreamControl, so it shouldn't be necessary to have commit/rollback call free() on all lobs to delete the temporary files. It would probably be enough if each lob added itself to a list in the connection each time a temporary file was created in LOBStreamControl.init(). (addLOBMapping() is probably OK, but it feels a bit strange to piggyback on a locator mechanism to do this cleanup when we don't actually care about the locators.)

        Show
        Knut Anders Hatlen added a comment - > I believe this is because EmbedConnection.addLobMapping is called for every lob creation but is never cleared until commit or rollback, even if the lob is freed. I believe your analysis is correct. free() should remove the mapping. Unfortunately, the lob doesn't know about its locator value, but it seems like your about to change that by adding a getLocator() method in DERBY-3243 . On DERBY-3243 I wrote: > Another thing I came to think about: Would it be better to remove addLOBMapping() from the constructors in EmbedBlob/EmbedClob and instead call it lazily from getLocator()? (...) Then we'd also remove the overhead of maintaining the lob mapping in embedded mode. I think that change would fix the OOME (on embedded, but we'd probably still see it in a client/server environment). Now, it seems like DERBY-2787 added the call to addLOBMapping() intentionally to ensure that free() was called on commit/rollback and temporary files would be deleted. Moving addLOBMapping() out of the constructors would probably reintroduce the problem with temporary files not being deleted. However, the lobs only create temporary files when they are modified and the new size of the lob is greater than the buffer in LOBStreamControl, so it shouldn't be necessary to have commit/rollback call free() on all lobs to delete the temporary files. It would probably be enough if each lob added itself to a list in the connection each time a temporary file was created in LOBStreamControl.init(). (addLOBMapping() is probably OK, but it feels a bit strange to piggyback on a locator mechanism to do this cleanup when we don't actually care about the locators.)
        Hide
        Øystein Grøvlen added a comment - - edited

        The attached patch makes the repro run without getting an OutOfMemory error.
        This is achieved by removing the LOB object from the locator mapping.

        I am not setting patch available since test cases should also be added,
        and I have not run the regression test suites.

        Show
        Øystein Grøvlen added a comment - - edited The attached patch makes the repro run without getting an OutOfMemory error. This is achieved by removing the LOB object from the locator mapping. I am not setting patch available since test cases should also be added, and I have not run the regression test suites.
        Hide
        Kathey Marsden added a comment -

        Thanks for the patch. I tried it out and it and have a few comments.

        1) In EmbedBlob should the call to localConn.removeLOBMapping(locator); be in a finally block like it is in EmbedClob?

        2) In jdk15 we don't have free() so would still have a leak. Do you have any suggestions for a jdk15 solution?

        3) I removed the call to free() from the repro and ran
        java LocLeak and got.
        Retrieving row 99000
        Exception in thread "main" java.sql.SQLException: Java exception: ': java.util.ConcurrentModificationException'.
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:95)
        at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:88)
        at org.apache.derby.impl.jdbc.Util.javaException(Util.java:245)
        at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:403)
        at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346)
        at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:1946)
        at org.apache.derby.impl.jdbc.EmbedConnection.rollback(EmbedConnection.java:1521)
        at LocLeak.main(LocLeak.java:14)
        Caused by: java.sql.SQLException: Java exception: ': java.util.ConcurrentModificationException'.
        at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45)
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:13
        5)
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:70)
        ... 7 more
        Caused by: java.util.ConcurrentModificationException
        at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793)
        at java.util.HashMap$ValueIterator.next(HashMap.java:821)
        at org.apache.derby.impl.jdbc.EmbedConnection.clearLOBMapping(EmbedConnection.java:2737)
        at org.apache.derby.impl.jdbc.EmbedConnection.rollback(EmbedConnection.java:1519)
        ... 1 more

        Show
        Kathey Marsden added a comment - Thanks for the patch. I tried it out and it and have a few comments. 1) In EmbedBlob should the call to localConn.removeLOBMapping(locator); be in a finally block like it is in EmbedClob? 2) In jdk15 we don't have free() so would still have a leak. Do you have any suggestions for a jdk15 solution? 3) I removed the call to free() from the repro and ran java LocLeak and got. Retrieving row 99000 Exception in thread "main" java.sql.SQLException: Java exception: ': java.util.ConcurrentModificationException'. at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:95) at org.apache.derby.impl.jdbc.Util.newEmbedSQLException(Util.java:88) at org.apache.derby.impl.jdbc.Util.javaException(Util.java:245) at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(TransactionResourceImpl.java:403) at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(TransactionResourceImpl.java:346) at org.apache.derby.impl.jdbc.EmbedConnection.handleException(EmbedConnection.java:1946) at org.apache.derby.impl.jdbc.EmbedConnection.rollback(EmbedConnection.java:1521) at LocLeak.main(LocLeak.java:14) Caused by: java.sql.SQLException: Java exception: ': java.util.ConcurrentModificationException'. at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:45) at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(SQLExceptionFactory40.java:13 5) at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:70) ... 7 more Caused by: java.util.ConcurrentModificationException at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793) at java.util.HashMap$ValueIterator.next(HashMap.java:821) at org.apache.derby.impl.jdbc.EmbedConnection.clearLOBMapping(EmbedConnection.java:2737) at org.apache.derby.impl.jdbc.EmbedConnection.rollback(EmbedConnection.java:1519) ... 1 more
        Hide
        Kathey Marsden added a comment -

        Also should the removeLobMapping calls be removed from LobStoredProcedures.BLOBRELEASELOCATOR and CLOBRELEASELOCATOR now that the release happens in free()?

        Show
        Kathey Marsden added a comment - Also should the removeLobMapping calls be removed from LobStoredProcedures.BLOBRELEASELOCATOR and CLOBRELEASELOCATOR now that the release happens in free()?
        Hide
        Øystein Grøvlen added a comment -

        Kathey, I agreed that more needs to be done for a final patch. I just put together a patch with the core fix to make it possible to try out the fix.

        To your issues:
        1) Creating a finally block in EmbedBlob seems like a good idea. I also agree that we can remove similar code from the stored procedures.

        2) I will think about solution for jdk15. The problem introduced by 10.3 is that the lob mapping keeps Blob/Clob objects alive that earlier would be gc'ed when it was no longer referred by the user. I guess another level of indirection would make it possible to use finalizers to clean up, but I am not sure that is a good idea.
        A work-around is to not create Blob objects in the first place, but use ResultSet#getBytes etc instead. So the problem is limited to the case where a Blob object need to be accessible after a call to ResultSet#next, but not until commit.

        3) Seems like there is come concurrent access to the HashMap. I would guess that this is because the iterator created by rollback, detects that the underlying collection has been changed. Probably through the calls to free in the loop using the iterator. I think this show that something needs to be reorganized a bit here.

        Show
        Øystein Grøvlen added a comment - Kathey, I agreed that more needs to be done for a final patch. I just put together a patch with the core fix to make it possible to try out the fix. To your issues: 1) Creating a finally block in EmbedBlob seems like a good idea. I also agree that we can remove similar code from the stored procedures. 2) I will think about solution for jdk15. The problem introduced by 10.3 is that the lob mapping keeps Blob/Clob objects alive that earlier would be gc'ed when it was no longer referred by the user. I guess another level of indirection would make it possible to use finalizers to clean up, but I am not sure that is a good idea. A work-around is to not create Blob objects in the first place, but use ResultSet#getBytes etc instead. So the problem is limited to the case where a Blob object need to be accessible after a call to ResultSet#next, but not until commit. 3) Seems like there is come concurrent access to the HashMap. I would guess that this is because the iterator created by rollback, detects that the underlying collection has been changed. Probably through the calls to free in the loop using the iterator. I think this show that something needs to be reorganized a bit here.
        Hide
        Anurag Shekhar added a comment -

        LobMapping and locater is used by the stored procedures (on behalf of
        network driver) to get hold of a particular lob object. LobMapping is additionally
        used to clear the lob object during commit/rollback and connection close.

        Separating these two functionality in two different collections may solve OOM problem.

        We can continue to have LobMapping only if the LOB is created through client driver (entry will
        be made in LOBMapiing in getLocator)

        Create methods will make an entry to a new collection holding week references of the LOBs.

        This will ensure that LOB will get garbage collected if they unreferenced in Embedded
        mode but in case of client server mode the references will be protected from being garbage
        collected unless free is called by the stored procedure.

        Show
        Anurag Shekhar added a comment - LobMapping and locater is used by the stored procedures (on behalf of network driver) to get hold of a particular lob object. LobMapping is additionally used to clear the lob object during commit/rollback and connection close. Separating these two functionality in two different collections may solve OOM problem. We can continue to have LobMapping only if the LOB is created through client driver (entry will be made in LOBMapiing in getLocator) Create methods will make an entry to a new collection holding week references of the LOBs. This will ensure that LOB will get garbage collected if they unreferenced in Embedded mode but in case of client server mode the references will be protected from being garbage collected unless free is called by the stored procedure.
        Hide
        Anurag Shekhar added a comment -

        In this patch I have introduced a WeakHashMap to hold lob objects.
        The WeakHashMap has lob objects as key and null as values.
        The existing lobMapping is used only if a locater is requested (ie when accessed from client driver).

        With this patch the attached test case (LocLeak.java) runs without getting out of memory exception (without free call from LocLeak).

        This patch has changes related to Clob only and I havn;t run any test other that attached LocLeak. I will submit a complete patch shortly.

        Show
        Anurag Shekhar added a comment - In this patch I have introduced a WeakHashMap to hold lob objects. The WeakHashMap has lob objects as key and null as values. The existing lobMapping is used only if a locater is requested (ie when accessed from client driver). With this patch the attached test case (LocLeak.java) runs without getting out of memory exception (without free call from LocLeak). This patch has changes related to Clob only and I havn;t run any test other that attached LocLeak. I will submit a complete patch shortly.
        Hide
        Øystein Grøvlen added a comment -

        If I understand correctly, the idea behind using a WeakHashMap is that if this is the only reference, it should not prevent the referred object from being garbage-collected.
        In other words, as long as a user thread or the network server through locators, refers to the object, it will not be garbage-collected, but if such references does not exist, the object will be garbage-collected. This is an interesting idea.

        My question is whether it is OK that free is not called on the LOB objects that are garbage-collected this way. Are you certain that all associated resources (e.g., temp files) will be released? Maybe some finalizer clean-up will also be needed?

        Other comments/questions:

        • It seems getLocator will create a new locator on every call. I think there should be just a single locator per object. Otherwise, I think the clean-up will be difficult.
        • free: Willl removeLOBMapping handle correctly the cases where a locator has not been set?
        • typo: Refrence
        Show
        Øystein Grøvlen added a comment - If I understand correctly, the idea behind using a WeakHashMap is that if this is the only reference, it should not prevent the referred object from being garbage-collected. In other words, as long as a user thread or the network server through locators, refers to the object, it will not be garbage-collected, but if such references does not exist, the object will be garbage-collected. This is an interesting idea. My question is whether it is OK that free is not called on the LOB objects that are garbage-collected this way. Are you certain that all associated resources (e.g., temp files) will be released? Maybe some finalizer clean-up will also be needed? Other comments/questions: It seems getLocator will create a new locator on every call. I think there should be just a single locator per object. Otherwise, I think the clean-up will be difficult. free: Willl removeLOBMapping handle correctly the cases where a locator has not been set? typo: Refrence
        Hide
        Anurag Shekhar added a comment -

        Thanks Øystein for look at the patch.

        My question is whether it is OK that free is not called on the LOB objects that are garbage-collected this way. Are you certain that all associated resources (e.g., temp files) will be released? Maybe some finalizer clean-up will also be needed?

        If there is no free called (or release on internal clob) the temporary file will not be cleaned. But the temporary file
        system is cleaned every time db boots up. I was under impression that disk operations are not allowed from
        finalize. Probably I was wrong. I will check and if its not restricted I will add finalizer for EmbedClob and
        EmbedBlob.

        Other comments/questions:

        • It seems getLocator will create a new locator on every call. I think there should be just a single locator per object. Otherwise, I think the clean-up will be difficult.

        I will be checking it to check if the locator is 0 before calling addMapping.

        • free: Willl removeLOBMapping handle correctly the cases where a locator has not been set?
          HashMap ignore remove call if key is not found in the Map So there is no problem If we call
          remove with invalid key.
        • typo: Refrence
          I will fix this,
        Show
        Anurag Shekhar added a comment - Thanks Øystein for look at the patch. My question is whether it is OK that free is not called on the LOB objects that are garbage-collected this way. Are you certain that all associated resources (e.g., temp files) will be released? Maybe some finalizer clean-up will also be needed? If there is no free called (or release on internal clob) the temporary file will not be cleaned. But the temporary file system is cleaned every time db boots up. I was under impression that disk operations are not allowed from finalize. Probably I was wrong. I will check and if its not restricted I will add finalizer for EmbedClob and EmbedBlob. Other comments/questions: It seems getLocator will create a new locator on every call. I think there should be just a single locator per object. Otherwise, I think the clean-up will be difficult. I will be checking it to check if the locator is 0 before calling addMapping. free: Willl removeLOBMapping handle correctly the cases where a locator has not been set? HashMap ignore remove call if key is not found in the Map So there is no problem If we call remove with invalid key. typo: Refrence I will fix this,
        Hide
        Øystein Grøvlen added a comment -

        I do think that it is a good idea to put disk operations in a finalizer, but it should be possible to record work that needs to be done (e.g., at transaction commit). I do not think delaying clean-up of files until db reboot is acceptable since a server may be running for months without a reboot.

        Show
        Øystein Grøvlen added a comment - I do think that it is a good idea to put disk operations in a finalizer, but it should be possible to record work that needs to be done (e.g., at transaction commit). I do not think delaying clean-up of files until db reboot is acceptable since a server may be running for months without a reboot.
        Hide
        Anurag Shekhar added a comment -

        Description of derby-3354_v1.diff
        This patch introduces a new WeakHashMap in EmbedConnection. EmbedBlob and EmbedClob objects references are stored in this
        map (objects as key and null as value). Adding entry to locater map is
        differed till the first call of getLocater.
        This ensures that there is entry of LOB objects in locater map if they are invoked in embedded mode.
        As the keys of WeakHashMap doesn't prevents the objects from being
        garbage collected, once the lob objects are unreferenced lob objects will
        be garbage collected releasing the memory.

        During commit/rollback or Connection.close, free is invoked on all the lob
        objects from WeakHashMap and the map is cleared.

        Modified files
        java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
        Added a new attribute lobRefrences of type WeakHashMap.
        Added a new method addLOBReference to make an entry in new
        hash map.
        Modified clearLOBMapping to use lobRefrences to fetch and invoke free on lob objects instead of lobHashMap.

        java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java
        java/engine/org/apache/derby/impl/jdbc/EmbedClob.java

        Modified constructs to call connection.lobRefrences instead of conn.addLOBMapping.
        Modified getLocater method to check if the locater value is non zero
        before returning and if its zero calling conn.addLOBMapping to make
        entry of lob objects and getting locater value.
        Calling removeLOBMapping in free method.

        Cleanup of temporary file is already being taken care by the finalizer of
        LOBStreamControl so I haven't added any new cleanup code for
        finalizer.

        lang and jdbcapi junit tests running clean with this patch applied. I am running rest of the test suite and will update the results of the same.

        Show
        Anurag Shekhar added a comment - Description of derby-3354_v1.diff This patch introduces a new WeakHashMap in EmbedConnection. EmbedBlob and EmbedClob objects references are stored in this map (objects as key and null as value). Adding entry to locater map is differed till the first call of getLocater. This ensures that there is entry of LOB objects in locater map if they are invoked in embedded mode. As the keys of WeakHashMap doesn't prevents the objects from being garbage collected, once the lob objects are unreferenced lob objects will be garbage collected releasing the memory. During commit/rollback or Connection.close, free is invoked on all the lob objects from WeakHashMap and the map is cleared. Modified files java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java Added a new attribute lobRefrences of type WeakHashMap. Added a new method addLOBReference to make an entry in new hash map. Modified clearLOBMapping to use lobRefrences to fetch and invoke free on lob objects instead of lobHashMap. java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java java/engine/org/apache/derby/impl/jdbc/EmbedClob.java Modified constructs to call connection.lobRefrences instead of conn.addLOBMapping. Modified getLocater method to check if the locater value is non zero before returning and if its zero calling conn.addLOBMapping to make entry of lob objects and getting locater value. Calling removeLOBMapping in free method. Cleanup of temporary file is already being taken care by the finalizer of LOBStreamControl so I haven't added any new cleanup code for finalizer. lang and jdbcapi junit tests running clean with this patch applied. I am running rest of the test suite and will update the results of the same.
        Hide
        Kristian Waagan added a comment -

        I haven't validated the approach, but it seems reasonable. I applied the patch and compiled Derby.

        Some very minor things you can consider changing:
        1) Typos: "refrences" -> "references" (5 occurrences)
        2) Use of a space between the method call / declaration and the following parenthesis is not consistent.
        3) Trailing whitespace at diff lines 28, 107 and 148.
        4) You could change the comment for the weak hash map to JavaDoc.

        Nice and small change
        Is this the final patch, or are the more coming up?

        Show
        Kristian Waagan added a comment - I haven't validated the approach, but it seems reasonable. I applied the patch and compiled Derby. Some very minor things you can consider changing: 1) Typos: "refrences" -> "references" (5 occurrences) 2) Use of a space between the method call / declaration and the following parenthesis is not consistent. 3) Trailing whitespace at diff lines 28, 107 and 148. 4) You could change the comment for the weak hash map to JavaDoc. Nice and small change Is this the final patch, or are the more coming up?
        Hide
        Anurag Shekhar added a comment -

        Thanks Kristian for looking at the patch.
        I will make the changes you have suggested and upload the patch.
        I am not planing for any more version of this patch (except for review changes.)

        Show
        Anurag Shekhar added a comment - Thanks Kristian for looking at the patch. I will make the changes you have suggested and upload the patch. I am not planing for any more version of this patch (except for review changes.)
        Hide
        Anurag Shekhar added a comment -

        in derby-3354_v2.diff I have fixed the issues Kristian had pointed out.

        junit suites runs without any failure wit this patch.

        Show
        Anurag Shekhar added a comment - in derby-3354_v2.diff I have fixed the issues Kristian had pointed out. junit suites runs without any failure wit this patch.
        Hide
        Øystein Grøvlen added a comment -

        Thanks for the patch Anurag. Comments:

        • I guess you need to do removeLOBMapping in EmbedBlob#free too.
        • Any resaon to keep references for already freed objects around until end of transaction?
        Show
        Øystein Grøvlen added a comment - Thanks for the patch Anurag. Comments: I guess you need to do removeLOBMapping in EmbedBlob#free too. Any resaon to keep references for already freed objects around until end of transaction?
        Hide
        Anurag Shekhar added a comment -

        Thanks Oystein for pointing out. I missed it, I will upload the patch after adding this.

        Show
        Anurag Shekhar added a comment - Thanks Oystein for pointing out. I missed it, I will upload the patch after adding this.
        Hide
        Anurag Shekhar added a comment -

        Added localConn.removeLOBMapping in Blob.free
        BlobClob4BlobTest runs fine with this change. I am
        running suites.All now.

        Show
        Anurag Shekhar added a comment - Added localConn.removeLOBMapping in Blob.free BlobClob4BlobTest runs fine with this change. I am running suites.All now.
        Hide
        Øystein Grøvlen added a comment -

        Is the removeLOBMapping put in the right plcae?
        Don't you have to do it regardless of whether it is materialized or not?
        Maybe it should have been put in a finally block like for EmbedClob? (Ref. Kathey's comment on my initial patch)

        Show
        Øystein Grøvlen added a comment - Is the removeLOBMapping put in the right plcae? Don't you have to do it regardless of whether it is materialized or not? Maybe it should have been put in a finally block like for EmbedClob? (Ref. Kathey's comment on my initial patch)
        Hide
        Anurag Shekhar added a comment -

        Putting remove mapping in else was a mistake. Sorry about that.
        I have moved it above other cleanup operation so that hash table is
        cleared irrespective of the result of other clean up.

        Show
        Anurag Shekhar added a comment - Putting remove mapping in else was a mistake. Sorry about that. I have moved it above other cleanup operation so that hash table is cleared irrespective of the result of other clean up.
        Hide
        Øystein Grøvlen added a comment -

        Thanks, for the updates, Anurag
        Patch v4 committed at revision 644764.

        Show
        Øystein Grøvlen added a comment - Thanks, for the updates, Anurag Patch v4 committed at revision 644764.
        Hide
        Øystein Grøvlen added a comment -

        It would be good if a test code be made based on the program that reproduced this error.

        Show
        Øystein Grøvlen added a comment - It would be good if a test code be made based on the program that reproduced this error.
        Hide
        Øystein Grøvlen added a comment -

        Fix merged to 10.4 branch at revision 644916.
        I am not setting this to resolve yet, since I think a test case for this should be added to the test suite.

        Show
        Øystein Grøvlen added a comment - Fix merged to 10.4 branch at revision 644916. I am not setting this to resolve yet, since I think a test case for this should be added to the test suite.
        Hide
        Kathey Marsden added a comment -

        I was thinking about whether to close this issue. Is there still a regression test coming?

        Show
        Kathey Marsden added a comment - I was thinking about whether to close this issue. Is there still a regression test coming?
        Hide
        Kathey Marsden added a comment -

        Going ahead and closing this issue. It would be good to have a regression test though.

        Show
        Kathey Marsden added a comment - Going ahead and closing this issue. It would be good to have a regression test though.

          People

          • Assignee:
            Anurag Shekhar
            Reporter:
            Kathey Marsden
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development