Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.4.0
    • Fix Version/s: 2.4.0
    • Labels:
      None
    • Environment:
      Java 8
      java version "1.8.0"
      Java(TM) SE Runtime Environment (build 1.8.0-b132)
      Java HotSpot(TM) Client VM (build 25.0-b70, mixed mode)

      Description

      One more test failure while running with Java 8. Since Java 8 introduced some new methods on the Map interface, this is probably related. We are getting a VerifyError related to the keySet signature:

      Tests in error:
      testConcurrentMap001(org.apache.openjpa.persistence.relations.TestConcurrentMap)

      <error message="(class: org/apache/openjpa/util/java$util$concurrent$ConcurrentHashMap$4$proxy, method: keySet signature: ()Ljava/util/concurrent/ConcurrentHashMap$KeySetView Wrong return type in function" type="java.lang.VerifyError">java.lang.VerifyError: (class: org/apache/openjpa/util/java$util$concurrent$ConcurrentHashMap$4$proxy, method: keySet signature: ()Ljava/util/concurrent/ConcurrentHashMap$KeySetView Wrong return type in function
      at java.lang.Class.forName0(Native Method)
      at java.lang.Class.forName(Class.java:340)
      at org.apache.openjpa.util.GeneratedClasses.loadBCClass(GeneratedClasses.java:68)

      1. OPENJPA-2492.patch
        4 kB
        Romain Manni-Bucau

        Issue Links

          Activity

          Hide
          romain.manni-bucau Romain Manni-Bucau added a comment - - edited

          we should surely only use ConcurrentMap (interface) and not concurrent hash map which has signature issues.

          wdyt?

          edit: an alternative is to implement ConcurrentMap ourself delegating to a ConcurrentHashMap (facade), this way signatures will be fine
          edit 2: looked deeper some tests and we rely on Map interfaces so we can just ignore this new keySet method from java 8 (+ I think that's ok from a spec point of view)

          Show
          romain.manni-bucau Romain Manni-Bucau added a comment - - edited we should surely only use ConcurrentMap (interface) and not concurrent hash map which has signature issues. wdyt? edit: an alternative is to implement ConcurrentMap ourself delegating to a ConcurrentHashMap (facade), this way signatures will be fine edit 2: looked deeper some tests and we rely on Map interfaces so we can just ignore this new keySet method from java 8 (+ I think that's ok from a spec point of view)
          Hide
          romain.manni-bucau Romain Manni-Bucau added a comment -

          patch which should make it fine with j8

          Show
          romain.manni-bucau Romain Manni-Bucau added a comment - patch which should make it fine with j8
          Hide
          kwsutter Kevin Sutter added a comment -

          Initial view of the patch looks good. In my experimentation, I had done something similar in ProxyManagerImpl. Except I had just checked for any return type that contained "KeySetView"... Something like this:

          if (meths[i].getReturnType().toString().contains("KeySetView")) continue; // KWS

          Don't know if this is any better or worse performing than your check, but at least we were on the same page.

          The one piece of your patch that I hadn't gotten to yet is beforeGet method in ProxyMaps:

          return map.keySet().contains(key);

          I think this was the key element of your patch. Thanks. The other changes in your patch seemed to be just window dressing. You cleaned up a conditional to use a method invocation, and you added a couple of asserts to a testcase. But, neither of these seemed critical to success. Agree?

          I'll take a look at the patch you posted for openjpa-2489 shortly... Maybe we're getting close to actually building JPA apps with Java 8. Thanks for your help!

          Show
          kwsutter Kevin Sutter added a comment - Initial view of the patch looks good. In my experimentation, I had done something similar in ProxyManagerImpl. Except I had just checked for any return type that contained "KeySetView"... Something like this: if (meths [i] .getReturnType().toString().contains("KeySetView")) continue; // KWS Don't know if this is any better or worse performing than your check, but at least we were on the same page. The one piece of your patch that I hadn't gotten to yet is beforeGet method in ProxyMaps: return map.keySet().contains(key); I think this was the key element of your patch. Thanks. The other changes in your patch seemed to be just window dressing. You cleaned up a conditional to use a method invocation, and you added a couple of asserts to a testcase. But, neither of these seemed critical to success. Agree? I'll take a look at the patch you posted for openjpa-2489 shortly... Maybe we're getting close to actually building JPA apps with Java 8. Thanks for your help!
          Hide
          romain.manni-bucau Romain Manni-Bucau added a comment -

          if you look j8 CHM implementation, containsKey does a get != null. So calling it from our proxy inproxied get method (beforeGet) creates an infinite loop -> keySet is a workaround. Using delegation would have made it cleaner but needs more refactoring.

          Show
          romain.manni-bucau Romain Manni-Bucau added a comment - if you look j8 CHM implementation, containsKey does a get != null. So calling it from our proxy inproxied get method (beforeGet) creates an infinite loop -> keySet is a workaround. Using delegation would have made it cleaner but needs more refactoring.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1589187 from Kevin Sutter in branch 'openjpa/trunk'
          [ https://svn.apache.org/r1589187 ]

          OPENJPA-2492. Committing a variation of the patch as provided by Romain Manni-Bucau. This patch clears up the ConcurrentHashMap issues relating to the KeySetView return type. This patch also adds a couple of asserts to an existing testcase to verify the results.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1589187 from Kevin Sutter in branch 'openjpa/trunk' [ https://svn.apache.org/r1589187 ] OPENJPA-2492 . Committing a variation of the patch as provided by Romain Manni-Bucau. This patch clears up the ConcurrentHashMap issues relating to the KeySetView return type. This patch also adds a couple of asserts to an existing testcase to verify the results.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1642566 from Jody Grassel in branch 'openjpa/branches/2.2.x'
          [ https://svn.apache.org/r1642566 ]

          OPENJPA-2492: TestConcurrentMap error with Java 8 [JDK8]

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1642566 from Jody Grassel in branch 'openjpa/branches/2.2.x' [ https://svn.apache.org/r1642566 ] OPENJPA-2492 : TestConcurrentMap error with Java 8 [JDK8]

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development