OpenJPA
  1. OpenJPA
  2. OPENJPA-646

JDK problems with defineClass and enum class types

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.2.1, 1.3.0
    • Component/s: kernel
    • Labels:
      None

      Description

      This Issue was first presented on our dev mailing list:

      http://www.nabble.com/Sun-JDK-problem-with-duplicate-annotations-td18101863.html

      I have also shared some additional experiences with Abe to get his views, but haven't heard anything back yet. So, I will create this JIRA Issue and work it from there.

      The original problem was limited to the Sun JDK. But, in a soon-to-be-available update to the IBM JDK, the same problem surfaces. At least the same exception is presented. The actual call stack is different due to the different Sun and IBM packages, but it's a similar problem.

      Basically, we are having a problem with our TempClassLoader attempting to use the defineClass() method on the CascadeType enum class. Our javax.persistence.CascadeType version doesn't experience this problem because we use Class.forName() to load the javax classes. In this scenario, I have an alt.persistence.CascadeType that falls into our normal class definition processing and attempts to use defineClass(). This method invocation completes okay. But, later when our code attempts to access the metadata associated with this type, we get the ArrayStoreException from the JDK.

      I will post both a simple project that reproduces the problem as well as a "hack" patch that gets us around the problem. I also have a PMR opened against the IBM JDK to see if they can help resolve it.

      Kevin

      1. pmr.zip
        245 kB
        Kevin Sutter
      2. openjpa-646.patch
        2 kB
        Kevin Sutter

        Issue Links

          Activity

          Hide
          Kevin Sutter added a comment -

          File pmr.zip is a simplified version of the problem that I put together for the IBM JDK problem report. To reproduce the problem, you have to build a jar file for this project and run the testcase with the -javaagent parameter. Or, you could just point the javaagent at the OpenJPA runtime jar file and use the "real" TempClassLoader implementation. Either way, you should be able to reproduce the problem.

          Show
          Kevin Sutter added a comment - File pmr.zip is a simplified version of the problem that I put together for the IBM JDK problem report. To reproduce the problem, you have to build a jar file for this project and run the testcase with the -javaagent parameter. Or, you could just point the javaagent at the OpenJPA runtime jar file and use the "real" TempClassLoader implementation. Either way, you should be able to reproduce the problem.
          Hide
          Kevin Sutter added a comment -

          This hack of a patch just modifies our TempClassLoader implementation to treat enum class types like annotation class types and use the Class.forName() mechanism to load the class instead of the defineClass() mechanism. Until I get educated on what exactly is wrong with our current processing, I don't want to commit this change. But, in case someone runs into this situation, this is a potential workaround.

          Show
          Kevin Sutter added a comment - This hack of a patch just modifies our TempClassLoader implementation to treat enum class types like annotation class types and use the Class.forName() mechanism to load the class instead of the defineClass() mechanism. Until I get educated on what exactly is wrong with our current processing, I don't want to commit this change. But, in case someone runs into this situation, this is a potential workaround.
          Hide
          Michael Dick added a comment -

          Moving to next release

          Show
          Michael Dick added a comment - Moving to next release
          Hide
          Kevin Sutter added a comment -

          I accidentally opened a new Issue for this same problem (OPENJPA-672). Just to get this Issue up to date, here are some of the details from that new Issue:

          From working with the JDK team, the problem is surfacing because the ClassLoader (AppClassLoader) that is used to load the Enum type when the alt.persistence.OneToMany is loaded is not the same ClassLoader (TemporaryClassLoader) when the Enum type is loaded by our enhancement processing. Thus, the AnnotationTypeMismatchExceptionProxy from the JDK.

          One way to workaround the problem is to add a test for Enum types in the TemporaryClassLoader and use the AppClassLoader in this case (much like we do for Annotation types):

          if (isAnnotation(classBytes) || isEnum(classBytes))
          return Class.forName(name, resolve, getClass().getClassLoader());

          The JDK team suggested removing the check for isAnnotation (and isEnum) altogether since that resolved the simple testcase that I had put together for their benefit. Unfortunately, that doesn't work for our enhancement processing for a couple of reasons. The _strats structure in PersistentMetaDataDefaults depended on the org.apache.openjpa.persistence.PersistentCollection class. By removing the conditional above, then we had a mismatch in classloaders for this data structure, much like the jdk problem.

          I tried changing the key for this _strats structure to use the class name string instead of the actual class instance. This got us around our immediate concern, but eventually I hit another JDK issue with mismatched classloaders when processing the annotations in AnnotationPersistentMetaDataParser:

          parsePersistentCollection(fmd, (PersistentCollection)
          el.getAnnotation(PersistentCollection.class));

          The loading of this PersistentCollection.class used the AppClassLoader, so the lookup via getAnnotation didn't find anything since the original annotation was loaded by the TemporaryClassLoader. Trying to get around this situation was creating some pretty ugly code.

          So, I am leaning towards the original workaround as a "solution" with proper commenting. By allowing the AppClassLoader to load enum types (vs the TemporaryClassLoader), we would "pollute" the AppClassLoader with left over enum classes. This would seem to be a minor drawback. Of course, if we ever need to allow for the enhancement of enum classes, then we're up a creek...

          Enough detail for now. Comments and suggestions are welcome.

          Show
          Kevin Sutter added a comment - I accidentally opened a new Issue for this same problem ( OPENJPA-672 ). Just to get this Issue up to date, here are some of the details from that new Issue: From working with the JDK team, the problem is surfacing because the ClassLoader (AppClassLoader) that is used to load the Enum type when the alt.persistence.OneToMany is loaded is not the same ClassLoader (TemporaryClassLoader) when the Enum type is loaded by our enhancement processing. Thus, the AnnotationTypeMismatchExceptionProxy from the JDK. One way to workaround the problem is to add a test for Enum types in the TemporaryClassLoader and use the AppClassLoader in this case (much like we do for Annotation types): if (isAnnotation(classBytes) || isEnum(classBytes)) return Class.forName(name, resolve, getClass().getClassLoader()); The JDK team suggested removing the check for isAnnotation (and isEnum) altogether since that resolved the simple testcase that I had put together for their benefit. Unfortunately, that doesn't work for our enhancement processing for a couple of reasons. The _strats structure in PersistentMetaDataDefaults depended on the org.apache.openjpa.persistence.PersistentCollection class. By removing the conditional above, then we had a mismatch in classloaders for this data structure, much like the jdk problem. I tried changing the key for this _strats structure to use the class name string instead of the actual class instance. This got us around our immediate concern, but eventually I hit another JDK issue with mismatched classloaders when processing the annotations in AnnotationPersistentMetaDataParser: parsePersistentCollection(fmd, (PersistentCollection) el.getAnnotation(PersistentCollection.class)); The loading of this PersistentCollection.class used the AppClassLoader, so the lookup via getAnnotation didn't find anything since the original annotation was loaded by the TemporaryClassLoader. Trying to get around this situation was creating some pretty ugly code. So, I am leaning towards the original workaround as a "solution" with proper commenting. By allowing the AppClassLoader to load enum types (vs the TemporaryClassLoader), we would "pollute" the AppClassLoader with left over enum classes. This would seem to be a minor drawback. Of course, if we ever need to allow for the enhancement of enum classes, then we're up a creek... Enough detail for now. Comments and suggestions are welcome.
          Hide
          Kevin Sutter added a comment -

          Resolved in the 1.2.x branch and 1.3.0 trunk.

          Show
          Kevin Sutter added a comment - Resolved in the 1.2.x branch and 1.3.0 trunk.
          Hide
          garpinc added a comment -

          Changing test class to an enum class results in linkage error in jdk 1.7 where it works in jdk 1.6. Need to re-address this behavior for jdk 1.7.

          Show
          garpinc added a comment - Changing test class to an enum class results in linkage error in jdk 1.7 where it works in jdk 1.6. Need to re-address this behavior for jdk 1.7.
          Hide
          Kevin Sutter added a comment -

          Are you indicating that the fix for this JIRA is related to the JDK 7 issue you are tracking with OpenJPA-2399? The last comment on the dev mailing list indicated that you couldn't get different results for JDK 6 and 7 with this testcase. Can you clarify what you changed and what you are experiencing now? I'm not questioning that some problems exists with JDK 7 usage, but I'm trying to figure out how or what aspect of this JIRA is related. Thanks.

          Show
          Kevin Sutter added a comment - Are you indicating that the fix for this JIRA is related to the JDK 7 issue you are tracking with OpenJPA-2399? The last comment on the dev mailing list indicated that you couldn't get different results for JDK 6 and 7 with this testcase. Can you clarify what you changed and what you are experiencing now? I'm not questioning that some problems exists with JDK 7 usage, but I'm trying to figure out how or what aspect of this JIRA is related. Thanks.
          Hide
          garpinc added a comment -

          Yes. I'm indicating that this issue is related to OpenJPA-2399. I discovered how to enable the javaagent aspect of the test case. This allowed me to first replicate the issue referenced in this JIRA and then I did the prescribed uncomment in the TemporaryClassLoader that resulted in the fix. Then I changed the test class to an enum class and I got the symptoms of OpenJPA-2399.. Feel free to contact me on skype with handle garpinc to follow up. I got diverted temporarily to a different project but now i'm back dealing with this issue again.

          Show
          garpinc added a comment - Yes. I'm indicating that this issue is related to OpenJPA-2399. I discovered how to enable the javaagent aspect of the test case. This allowed me to first replicate the issue referenced in this JIRA and then I did the prescribed uncomment in the TemporaryClassLoader that resulted in the fix. Then I changed the test class to an enum class and I got the symptoms of OpenJPA-2399.. Feel free to contact me on skype with handle garpinc to follow up. I got diverted temporarily to a different project but now i'm back dealing with this issue again.

            People

            • Assignee:
              Kevin Sutter
              Reporter:
              Kevin Sutter
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development