castor
  1. castor
  2. CASTOR-3065

ArrayIndexOutOfBoundsException occurs when loading an extended object

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.3rc1
    • Fix Version/s: 1.3.3rc1
    • Component/s: JDO
    • Labels:
      None
    • Testcase included:
      yes
    • Number of attachments :
      5

      Description

      Suppose class A, B and C, and A extends B, B extends C, and an object with key 1 is an instance of Class C.
      Now we load (B, 1), there is an ArrayIndexOutOfBoundsException. But we load(A, 1) and load(C, 1) will work fine.
      Please see some tests (a patch, attachment) which show the error.

      1. load_extended_object_error_patch.txt
        35 kB
        Wensheng Dou
      2. load_extended_object_error_patch2.txt
        2 kB
        Wensheng Dou
      3. load_extended_object_error_patch3.txt
        26 kB
        Wensheng Dou
      4. load_extended_object_error_test_patch.txt
        15 kB
        Wensheng Dou
      5. patch-C3065-20110512.txt
        37 kB
        Ralf Joachim

        Activity

        Hide
        Wensheng Dou added a comment - - edited

        I think the error is caused by the acquire method int the TypeInfo class (originating from LockEngine class). When it judges whether it can fing a base class in cache, it should judge all the superclasses.
        A small patch for this (see attachment load_extended_object_error_patch.txt).

        Show
        Wensheng Dou added a comment - - edited I think the error is caused by the acquire method int the TypeInfo class (originating from LockEngine class). When it judges whether it can fing a base class in cache, it should judge all the superclasses. A small patch for this (see attachment load_extended_object_error_patch.txt).
        Hide
        Ralf Joachim added a comment -

        Hi Wensheng,

        while I think it is a good idea to move TypeInfo into a separate class, it makes the changes you made to fix this issue very hard to find. Could you please isolate the 2 changes into separate issues and patches.

        Regards
        Ralf

        Show
        Ralf Joachim added a comment - Hi Wensheng, while I think it is a good idea to move TypeInfo into a separate class, it makes the changes you made to fix this issue very hard to find. Could you please isolate the 2 changes into separate issues and patches. Regards Ralf
        Hide
        Wensheng Dou added a comment -

        Hi Ralf,

        Thanks for your sugguestion. I have modified the patch, see attachment (load_extended_object_error_patch2.txt)

        Best regards,
        Wensheng

        Show
        Wensheng Dou added a comment - Hi Ralf, Thanks for your sugguestion. I have modified the patch, see attachment (load_extended_object_error_patch2.txt) Best regards, Wensheng
        Hide
        Ralf Joachim added a comment -

        Hi Wensheng,

        I have had a look at your patchs and have 3 things for you to consider.

        1. OID.getSuperClassNames() gets only used at your patch. If this method would return a List instead of an array the required check at LockEngine.TypeInfo.aquire() would be a bit simpler as we would only need to call oid.getSuperClassNames().contains(...). In addition we could change _superClassNames property of OID to List also. This would also make OID constructor simpler. Apart of this improvement your patch works and is absolutely reasonable.

        2. The tests of cpactf which had been migrated from our previous test framework have numbers without any meaning. Tests added later usually get the number of the jira issue they had been added with. I would therefore suggest to rename everything of the test to 'test3065' instead of 'test1159'.

        3. Could you please setup checkstyle. If you do this you will see that your patch contains 2 violations of our codeing conventions. Having said that you will also see a lot of other violations. You do not have to fix all of them but we like no new ones to be introduced. See http://eclipse-cs.sourceforge.net/downloads.html

        Regards
        Ralf

        Show
        Ralf Joachim added a comment - Hi Wensheng, I have had a look at your patchs and have 3 things for you to consider. 1. OID.getSuperClassNames() gets only used at your patch. If this method would return a List instead of an array the required check at LockEngine.TypeInfo.aquire() would be a bit simpler as we would only need to call oid.getSuperClassNames().contains(...). In addition we could change _superClassNames property of OID to List also. This would also make OID constructor simpler. Apart of this improvement your patch works and is absolutely reasonable. 2. The tests of cpactf which had been migrated from our previous test framework have numbers without any meaning. Tests added later usually get the number of the jira issue they had been added with. I would therefore suggest to rename everything of the test to 'test3065' instead of 'test1159'. 3. Could you please setup checkstyle. If you do this you will see that your patch contains 2 violations of our codeing conventions. Having said that you will also see a lot of other violations. You do not have to fix all of them but we like no new ones to be introduced. See http://eclipse-cs.sourceforge.net/downloads.html Regards Ralf
        Hide
        Wensheng Dou added a comment -

        Hi Ralf,
        Thank your for your suggestion. I have installed the checkstyle, and have done as your suggested. If you have time, please check them again (see attachment: load_extended_object_error_patch3.txt)

        Thanks.

        Show
        Wensheng Dou added a comment - Hi Ralf, Thank your for your suggestion. I have installed the checkstyle, and have done as your suggested. If you have time, please check them again (see attachment: load_extended_object_error_patch3.txt) Thanks.
        Hide
        Ralf Joachim added a comment -

        Final patch with some minor improvements:

        • fixed typo ('test3056' -> 'test3065')
        • added scripts for mysql, oracle and postgresql
        • tested against derby, mysql, oracle and postgresql
        • updated test matrix
        • added release notes
        Show
        Ralf Joachim added a comment - Final patch with some minor improvements: fixed typo ('test3056' -> 'test3065') added scripts for mysql, oracle and postgresql tested against derby, mysql, oracle and postgresql updated test matrix added release notes

          People

          • Assignee:
            Ralf Joachim
            Reporter:
            Wensheng Dou
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: